On Fri, Apr 24, 2020 at 2:51 PM Xin Ji xji@analogixsemi.com wrote:
On Thu, Apr 23, 2020 at 07:55:15PM +0800, Nicolas Boichat wrote:
Hi,
Just commenting on the mode_fixup function that was added in v7.
[snip]
/** once illegal timing detected, use default HFP, HSYNC, HBP*/if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to adj_hsync, adj_hfp, adj_hbp.
adj_hsync = SYNC_LEN_DEF;adj_hfp = HFP_HBP_DEF;adj_hbp = HFP_HBP_DEF;vref = adj->clock * 1000 / (adj->htotal * adj->vtotal);if (hblanking < HBLANKING_MIN) {delta_adj = HBLANKING_MIN - hblanking;adj_clock = vref * delta_adj * adj->vtotal;adj->clock += DIV_ROUND_UP(adj_clock, 1000);} else {delta_adj = hblanking - HBLANKING_MIN;adj_clock = vref * delta_adj * adj->vtotal;adj->clock -= DIV_ROUND_UP(adj_clock, 1000);}DRM_WARN("illegal hblanking timing, use default.\n");DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.