[Intel-xe] [PATCH v3 02/20] drm/xe: Use managed pci_enable_device

[Michał Winiarski]

On Wed, Nov 15, 2023 at 12:47:00PM -0800, Matt Roper wrote:
> On Tue, Nov 14, 2023 at 02:02:13PM +0100, Michał Winiarski wrote:
> > Xe uses devres for most of its driver-lifetime resources, use it for pci
> > device as well.
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> > ---
> > v2 -> v3:
> > - Mark xe_pci_clear_master as static to fix W=1 build warning (CI)
> > 
> >  drivers/gpu/drm/xe/xe_pci.c | 26 ++++++++++++++------------
> >  1 file changed, 14 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index de986aaee3bb2..e6b59c4b02bdc 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -663,6 +663,11 @@ static void xe_pci_remove(struct pci_dev *pdev)
> >  	pci_set_drvdata(pdev, NULL);
> >  }
> >  
> > +static void xe_pci_clear_master(void *pdev)
> > +{
> > +	pci_clear_master(pdev);
> > +}
> > +
> >  static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  {
> >  	const struct xe_device_desc *desc = (const void *)ent->driver_data;
> > @@ -691,23 +696,25 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	if (xe_display_driver_probe_defer(pdev))
> >  		return -EPROBE_DEFER;
> >  
> > +	err = pcim_enable_device(pdev);
> > +	if (err)
> > +		return err;
> > +
> >  	xe = xe_device_create(pdev, ent);
> >  	if (IS_ERR(xe))
> >  		return PTR_ERR(xe);
> >  
> > +	pci_set_drvdata(pdev, xe);
> > +
> >  	xe_pm_assert_unbounded_bridge(xe);
> >  	subplatform_desc = find_subplatform(xe, desc);
> >  
> > -	pci_set_drvdata(pdev, xe);
> > -	err = pci_enable_device(pdev);
> > -	if (err)
> > -		return err;
> > -
> >  	pci_set_master(pdev);
> > +	devm_add_action(&pdev->dev, xe_pci_clear_master, pdev);
> 
> Is it necessary to do this explicitly?  I think this already happens in
> pci_disable_device -> do_pci_disable_device, right?  I don't see any
> other drivers adding an explicit managed action to do it either.

You're right - it can be removed.

Thanks,
-Michał

> 
> 
> Matt
> 
> >  
> >  	err = xe_info_init(xe, desc, subplatform_desc);
> >  	if (err)
> > -		goto err_pci_disable;
> > +		return err;
> >  
> >  	xe_display_probe(xe);
> >  
> > @@ -734,16 +741,11 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  
> >  	err = xe_device_probe(xe);
> >  	if (err)
> > -		goto err_pci_disable;
> > +		return err;
> >  
> >  	xe_pm_init(xe);
> >  
> >  	return 0;
> > -
> > -err_pci_disable:
> > -	pci_disable_device(pdev);
> > -
> > -	return err;
> >  }
> >  
> >  static void xe_pci_shutdown(struct pci_dev *pdev)
> > -- 
> > 2.42.1
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation