Re: [PATCH v2 13/28] drm: omapdrm: dss: Initialize DSS internal features at probe time

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

On Tuesday 09 May 2017 12:48:57 Tomi Valkeinen wrote:
> On 08/05/17 14:32, Laurent Pinchart wrote:
> > The DSS internal features are derived from the platform device
> > compatible string which is available at probe time. Don't delay
> > initialization until bind time. This prepares for the merge of the two
> > DSS features structures that will be needed before the DSS is bound.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/omapdrm/dss/dss.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c
> > b/drivers/gpu/drm/omapdrm/dss/dss.c index 7546ba8ab955..4fdb77dd90b8
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> > @@ -1183,23 +1183,10 @@ static const struct soc_device_attribute
> > dss_soc_devices[] = {> 
> >  static int dss_bind(struct device *dev)
> >  {
> >  
> >  	struct platform_device *pdev = to_platform_device(dev);
> > 
> > -	const struct soc_device_attribute *soc;
> > 
> >  	struct resource *dss_mem;
> >  	u32 rev;
> >  	int r;
> > 
> > -	dss.pdev = pdev;
> > -
> > -	/*
> > -	 * The various OMAP3-based SoCs can be told apart from the compatible
> > -	 * string, use SoC device matching.
> > -	 */
> > -	soc = soc_device_match(dss_soc_devices);
> > -	if (soc)
> > -		dss.feat = soc->data;
> > -	else
> > -		dss.feat = of_match_device(dss_of_match, &pdev->dev)->data;
> > -
> > 
> >  	dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0);
> >  	dss.base = devm_ioremap_resource(&pdev->dev, dss_mem);
> >  	if (!dss.base)
> > 
> > @@ -1331,9 +1318,22 @@ static int dss_add_child_component(struct device
> > *dev, void *data)> 
> >  static int dss_probe(struct platform_device *pdev)
> >  {
> > 
> > +	const struct soc_device_attribute *soc;
> > 
> >  	struct component_match *match = NULL;
> >  	int r;
> > 
> > +	dss.pdev = pdev;
> > +
> > +	/*
> > +	 * The various OMAP3-based SoCs can be told apart from the compatible
> > +	 * string, use SoC device matching.
> > +	 */
> > +	soc = soc_device_match(dss_soc_devices);
> > +	if (soc)
> > +		dss.feat = soc->data;
> > +	else
> > +		dss.feat = of_match_device(dss_of_match, &pdev->dev)->data;
> > +
> 
> This has the problem that we should remove the static 'dss' variable,
> and allocate memory for each device. We can have more than one DSS in
> the future.

Sure, and that's on my to-do list, but I didn't want to make the patch series 
bigger than it is already.

> Also, I haven't looked at the following patches yet, but dss_features
> was a failed attempt to manage the dss features. Since then, we've
> slowly been moving the bits and pieces from dss_features to each
> individual driver.
> 
> So if this is needed for dss_features, we should instead move everything
> from dss_features and remove that altogether.

I agree with you, but again, I've tried to keep the existing architecture 
here, we can certainly rework it as a next step.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel