Re: [PATCH v2 3/3] drm/panel: simple: add CMT430B19N00 LCD panel support

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]

On Tue, Mar 05, 2024 at 10:46:55AM +0100, Jérémie Dautheribes wrote:
> Hi Maxime,
> 
> On 04/03/2024 17:25, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Mar 04, 2024 at 05:04:54PM +0100, Jérémie Dautheribes wrote:
> > > Add support for Crystal Clear Technology CMT430B19N00 4.3" 480x272
> > > TFT-LCD panel.
> > > 
> > > Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> > > ---
> > >   drivers/gpu/drm/panel/panel-simple.c | 29 ++++++++++++++++++++++++++++
> > >   1 file changed, 29 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > index 20e3df1c59d4..b940220f56e2 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -1457,6 +1457,32 @@ static const struct panel_desc boe_hv070wsa = {
> > >   	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >   };
> > > +static const struct drm_display_mode cct_cmt430b19n00_mode = {
> > > +	.clock = 9000,
> > > +	.hdisplay = 480,
> > > +	.hsync_start = 480 + 43,
> > > +	.hsync_end = 480 + 43 + 8,
> > > +	.htotal = 480 + 43 + 8 + 4,
> > > +	.vdisplay = 272,
> > > +	.vsync_start = 272 + 12,
> > > +	.vsync_end = 272 + 12 + 8,
> > > +	.vtotal = 272 + 12 + 8 + 4,
> > > +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> > > +};
> > 
> > Your pixel clock doesn't really match the rest of the timings:
> > 
> > (480 + 43 + 8 + 4) * (272 + 12 + 8 + 4) * 60 = 9501600
> > 
> > So a ~6% deviation.
> > 
> > What does the datasheet say?
> 
> Indeed it does not exactly match but the datasheet indicates that the
> typical clock frequency is 9MHz and when this frequency is used, the
> typical values of the other parameters are those we have defined in
> the drm_display_mode structure.

It seems weird to me that all the typical timings end up in a
non-typical configuration, but I've seen my fair share of weird
datasheet, so.. yeah.

I guess the best thing to do if you have access to the min/typ/max
timings is to actually use the display_timings structure here and define
all of them.

It at least gives us the opportunity to fix it later on.

> I don't see any information about the accepted deviation either.

It's not only about the panel itself. 6% gives your roughly 56fps when
you meant 60. This can then trip up some applications too. Like if
you're playing a 60fps application, it will either play too fast or
you'll get stutter, depending on how the video playback has been
implemented exactly.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]