Re: [PATCH 24/33] drm/panel-simple: Fix dotclock for Ortustech COM37H3M

Ville Syrjälä
2020-03-03T15:03:42Z

On Mon, Mar 02, 2020 at 10:24:14PM +0100, H. Nikolaus Schaller wrote:
> Hi Ville,
>
> > Am 02.03.2020 um 21:34 schrieb Ville Syrjala <ville.syrjala@linux.intel.com>:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The currently listed dotclock disagrees with the currently
> > listed vrefresh rate. Change the dotclock to match the vrefresh.
> >
> > Someone tell me which (if either) of the dotclock or vreresh is
> > correct?
>
> Data sheet of COM37H3M99DTC says:
>
> MIN TYP MAX
> CLK frequency fCLK 18 19.8 27 MHz
> VSYNC Frequency fVSYNC 54 60 66 Hz
> VSYNC cycle time tv 646 650 700 H
> HSYNC frequency fHSYNC -- 39.0 50.0 Khz
> HSYNC cycle time th 504 508 630 CLK
>
> But data sheet of COM37H3M05DTC says
>
> MIN TYP MAX
> CLK frequency fCLK -- 22.4 26.3 MHz (in VGA mode - there is also an QVGA mode)
> VSYNC Frequency fVSYNC 54 60 66 Hz
> VSYNC cycle time tv -- 650 -- H
> HSYNC frequency fHSYNC -- 39.3 -- Khz
> HSYNC cycle time th -- 570 -- CLK
>
> So there are two different subvariants of the same panel.
>
> If I remember correctly, the 05 is older (April 2010)
> and the 99DTC newer (Dec 2011).
>
> So 22 MHz isn't outside of either spec but may be higher
> than needed for the 99DTC. I.e. the panel is probably
> running at higher frame rate than 60 fps.
>
> Hm. I think we should define some compromise. I.e.
>
> .clock = 22230
> .vrefresh = 60
> .vtotal = 650
> .htotal = 570
>
> Probably we originally tried to do this with the parameter
> set but got something wrong.
>
> If you agree with this approach, I can try to figure out
> the other parameters so that they should fit both panel
> variants. I can only test with COM37H3M99DTC since I
> do no longer have a device with COM37H3M05DTC.
>
> In general it seems that the structure drm_display_mode
> is overdetermined.
>
> Either .clock could be calculated from .vrefresh (and
> the other .vtotal and .htotal) or vice versa like I
> did for the proposal above.
>
> I haven't looked into the driver code, but would it be
> possible to specify .clock = 0 (or leave it out) to
> calculate it bottom up? This would avoid such inconsistencies.

I'm going to remove .vrefresh entirely from the struct.
It'll just be calculated from the other timings as needed.

>
> On the other hand it is not easily visible any more
> from the code if the clock is in range of the data
> sheet limits.
>
> BR and thanks,
> Nikolaus
>
> >
> > Cc: H. Nikolaus Schaller <hns@goldelico.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/panel/panel-simple.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index ca72b73408e9..f9b4f84bffb3 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -2617,7 +2617,7 @@ static const struct panel_desc ontat_yx700wv03 = {
> > };
> >
> > static const struct drm_display_mode ortustech_com37h3m_mode = {
> > - .clock = 22153,
> > + .clock = 19842,
> > .hdisplay = 480,
> > .hsync_start = 480 + 8,
> > .hsync_end = 480 + 8 + 10,
> > --
> > 2.24.1
> >

--
Ville Syrjälä
Intel