Re: [PATCH] drm: panel: simple: Drop drive/sample bus flags for LVDS panels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 28, 2020 at 02:48:20PM +0300, Laurent Pinchart wrote:
> Hi Sam,
> 
> On Sun, Jun 28, 2020 at 11:13:47AM +0200, Sam Ravnborg wrote:
> > On Sun, Jun 28, 2020 at 10:54:01AM +0300, Laurent Pinchart wrote:
> > > The DRM bus flags reporting on which clock edge the pixel data and sync
> > > signals are sampled or driven don't make sense for LVDS panels, as the
> > > bus then uses sub-clock timings to send data. Drop those flags and add a
> > > warning in the probe function to make sure the mistake won't be
> > > repeated.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> > > ---
> > >  drivers/gpu/drm/panel/panel-simple.c | 30 +++++++++++++++++-----------
> > >  1 file changed, 18 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > index bff1cab529d2..537d75ef7fc2 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -549,6 +549,14 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
> > >  			panel_simple_parse_panel_timing_node(dev, panel, &dt);
> > >  	}
> > >  
> > > +	if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS)
> > > +		/* Catch common mistakes for LVDS panels. */
> > > +		WARN_ON(desc->bus_flags &
> > > +			(DRM_BUS_FLAG_PIXDATA_POSEDGE |
> > > +			 DRM_BUS_FLAG_PIXDATA_NEGEDGE |
> > > +			 DRM_BUS_FLAG_SYNC_POSEDGE |
> > > +			 DRM_BUS_FLAG_SYNC_NEGEDGE));
> >
> > The above are only the legacy flags.
> 
> We currently have
> 
> 	DRM_BUS_FLAG_PIXDATA_POSEDGE = BIT(2),
> 	DRM_BUS_FLAG_PIXDATA_NEGEDGE = BIT(3),
> 	DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> 	DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> 	DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> 	DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> 
> Only the first two are legacy, but the last four have the same values. I
> could pick DRIVE or SAMPLE (the latter seems to be a better match here)
> instead.
We want to get rid of the legacy values, so yes, no more new users.

	Sam

> 
> > For LVDS panels I think we could check if either
> > DRM_BUS_FLAG_DE_LOW or DRM_BUS_FLAG_DE_HIGH is set and nothing else.
> 
> A whitelist instead of a blacklist is a good idea.
> 
> > And then as an additional check that the .bus_format is relevant.
> 
> I have a patch for that, but we first need to fix two occurrences of
> incorrect formats. As I couldn't find documentation for these two
> panels, I've replied to the patches that introduced them, and am waiting
> for answers.
> 
> > Then we have managed to catch most of the mistakes for LVDS panels.
> > 
> > We could add checks for DPI panels too, but that would be later.
> > 
> > > +
> > >  	drm_panel_init(&panel->base, dev, &panel_simple_funcs,
> > >  		       desc->connector_type);
> > >  
> > > @@ -688,7 +696,7 @@ static const struct panel_desc auo_b101aw03 = {
> > >  		.height = 125,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -1398,7 +1406,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = {
> > >  		.height = 150,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -1423,7 +1431,7 @@ static const struct panel_desc chunghwa_claa101wa01a = {
> > >  		.height = 120,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -1448,7 +1456,7 @@ static const struct panel_desc chunghwa_claa101wb01 = {
> > >  		.height = 125,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -2123,7 +2131,7 @@ static const struct panel_desc innolux_n156bge_l21 = {
> > >  		.height = 193,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -2256,7 +2264,7 @@ static const struct panel_desc koe_tx26d202vm0bwa = {
> > >  		.disable = 1000,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -2497,9 +2505,7 @@ static const struct panel_desc logictechno_lt170410_2whc = {
> > >  		.height = 136,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH |
> > > -		     DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
> > > -		     DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -3072,7 +3078,7 @@ static const struct panel_desc samsung_ltn101nt05 = {
> > >  		.height = 125,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -3319,7 +3325,7 @@ static const struct panel_desc starry_kr070pe2t = {
> > >  		.height = 86,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> > > @@ -3512,7 +3518,7 @@ static const struct panel_desc toshiba_lt089ac29000 = {
> > >  		.height = 116,
> > >  	},
> > >  	.bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> > >  	.connector_type = DRM_MODE_CONNECTOR_LVDS,
> > >  };
> > >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux