[PATCH v3 02/21] drm/panel: panel-simple: add default connector_type

[Sam Ravnborg]

Hi Laurent.
On Sat, Jul 11, 2020 at 01:11:24AM +0300, Laurent Pinchart wrote:
> Hi Sam,
> 
> Thank you for the patch.
> 
> On Fri, Jul 03, 2020 at 09:23:58PM +0200, Sam Ravnborg wrote:
> > All panels shall report a connector type.
> > panel-simple has a lot of panels with no connector_type,
> > and for these fall back to DPI as the default.
> > 
> > Signed-off-by: Sam Ravnborg <sam at ravnborg.org>
> > Cc: Thierry Reding <thierry.reding at gmail.com>
> > Cc: Sam Ravnborg <sam at ravnborg.org>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index 7b5d212215e0..b3ec965721b0 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -500,6 +500,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
> >  	struct panel_simple *panel;
> >  	struct display_timing dt;
> >  	struct device_node *ddc;
> > +	int connector_type;
> >  	int err;
> >  
> >  	panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
> > @@ -566,8 +567,13 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
> >  			desc->bpc != 8);
> >  	}
> >  
> > -	drm_panel_init(&panel->base, dev, &panel_simple_funcs,
> > -		       desc->connector_type);
> > +	/* Default DRM_MODE_CONNECTOR_DPI if no connector_type is set */
> > +	if (desc->connector_type != 0)
> > +		connector_type = desc->connector_type;
> > +	else
> > +		connector_type = DRM_MODE_CONNECTOR_DPI;
> 
> This avoids a WARN_ON(), which is good, but should we add a dev_warn()
> to get this fixed ?

We need a proper check for all relevant properties for DPI
and the other connectors. Like you already did for LVDS.

The idea is we shall introduce the checks in one series,
so users when they fix the warnings they are all good.

And then we will have to catch less during review which is good.

It is on my TODO list, but wanted to have some other stuff done
first.

So in other words, good to go with this patch or do we need the proper
checks in place first?

	Sam

> 
> > +
> > +	drm_panel_init(&panel->base, dev, &panel_simple_funcs, connector_type);
> >  
> >  	err = drm_panel_of_backlight(&panel->base);
> >  	if (err)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel