Hi Philip,
On Thu, 28 May 2015 13:13:28 +0200
Philipp Zabel <p.zabel at pengutronix.de> wrote:
> Hi Boris,
>
> Am Dienstag, den 26.05.2015, 11:28 +0200 schrieb Boris Brezillon:
> > Hi David,
> >
> > On Thu, 21 May 2015 11:06:56 +0200
> > David Dueck <davidcdueck at googlemail.com> wrote:
> >
> > > drm_panel supports querying timing ranges. If the supplied mode does
> > > not work with the hlcdc we query the panel and try to find a suitable
> > > mode.
> >
> > This patch looks good to me.
> >
> > Philip, Thierry, could you confirm this is the correct way of dealing
> > with timing ranges.
>
> I wonder about two things:
>
> This implementation minimizes the sum of absolute differences between
> chosen and typical values. I wonder if it would be better to try and
> minimize the difference between the chosen and nominal vertical refresh
> rate.
I'm not sure to understand what you mean.
Are you suggesting that we should try keeping the vtotal (and maybe the
htotal too) value unchanged by adapting the timing values ?
Something like that:
vfront_porch = atmel_hlcdc_choose_parameter(1, 0x40,
timings->vfront_porch);
adjusted_mode->vsync_start = adjusted_mode->vdisplay + vfront_porch;
vsync_len = atmel_hlcdc_choose_parameter(1, 0x40,
adjusted_mode->vsync_end -
adjusted_mode->vsync_start);
adjusted_mode->vsync_end = adjusted_mode->vsync_start + vsync_len;
vback_porch = atmel_hlcdc_choose_parameter(0, 0x40,
timings->vtotal -
adjusted_mode->vsync_end);
adjusted_mode->vtotal = adjusted_mode->vsync_end + vback_porch;
/* ... */
If that's the case, then I definitely agree.
>
> Is this something that should be done earlier, in create_panel_output,
> so that the connector's modes already contain the corrected settings?
You mean creating our own drm_display modes (and taking the timing
constraints when creating those modes) instead of calling drm_panel's
->get_modes().
That sounds reasonable. David, what's your opinion.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
