Re: [PATCH 5/5] drm/vboxvideo: Use managed interfaces for framebuffer write combining

[Daniel Vetter]

On Thu, Sep 16, 2021 at 09:28:53PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 9/16/21 8:16 PM, Thomas Zimmermann wrote:
> > Replace arch_phys_wc_add() with the rsp managed function. Allows for
> > removing the cleanup code for memory management
> > 
> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> 
> Thanks, patch looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Maybe review entire series and then ask Thomas to unblock some of your
stuff?
-Daniel

> 
> Regards,
> 
> Hans
> 
> 
> > ---
> >  drivers/gpu/drm/vboxvideo/vbox_drv.c |  5 +----
> >  drivers/gpu/drm/vboxvideo/vbox_drv.h |  1 -
> >  drivers/gpu/drm/vboxvideo/vbox_ttm.c | 17 ++++++++---------
> >  3 files changed, 9 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > index 2b81cb259d23..a6c81af37345 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > @@ -69,7 +69,7 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  
> >  	ret = vbox_mode_init(vbox);
> >  	if (ret)
> > -		goto err_mm_fini;
> > +		goto err_hw_fini;
> >  
> >  	ret = vbox_irq_init(vbox);
> >  	if (ret)
> > @@ -87,8 +87,6 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	vbox_irq_fini(vbox);
> >  err_mode_fini:
> >  	vbox_mode_fini(vbox);
> > -err_mm_fini:
> > -	vbox_mm_fini(vbox);
> >  err_hw_fini:
> >  	vbox_hw_fini(vbox);
> >  	return ret;
> > @@ -101,7 +99,6 @@ static void vbox_pci_remove(struct pci_dev *pdev)
> >  	drm_dev_unregister(&vbox->ddev);
> >  	vbox_irq_fini(vbox);
> >  	vbox_mode_fini(vbox);
> > -	vbox_mm_fini(vbox);
> >  	vbox_hw_fini(vbox);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > index 4903b91d7fe4..e77bd6512eb1 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > @@ -139,7 +139,6 @@ void vbox_mode_fini(struct vbox_private *vbox);
> >  void vbox_report_caps(struct vbox_private *vbox);
> >  
> >  int vbox_mm_init(struct vbox_private *vbox);
> > -void vbox_mm_fini(struct vbox_private *vbox);
> >  
> >  /* vbox_irq.c */
> >  int vbox_irq_init(struct vbox_private *vbox);
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_ttm.c b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> > index fd8a53a4d8d6..dc24c2172fd4 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> > @@ -13,22 +13,21 @@
> >  int vbox_mm_init(struct vbox_private *vbox)
> >  {
> >  	int ret;
> > +	resource_size_t base, size;
> >  	struct drm_device *dev = &vbox->ddev;
> >  	struct pci_dev *pdev = to_pci_dev(dev->dev);
> >  
> > -	ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0),
> > -				       vbox->available_vram_size);
> > +	base = pci_resource_start(pdev, 0);
> > +	size = pci_resource_len(pdev, 0);
> > +
> > +	/* Don't fail on errors, but performance might be reduced. */
> > +	devm_arch_phys_wc_add(&pdev->dev, base, size);
> > +
> > +	ret = drmm_vram_helper_init(dev, base, vbox->available_vram_size);
> >  	if (ret) {
> >  		DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
> >  		return ret;
> >  	}
> >  
> > -	vbox->fb_mtrr = arch_phys_wc_add(pci_resource_start(pdev, 0),
> > -					 pci_resource_len(pdev, 0));
> >  	return 0;
> >  }
> > -
> > -void vbox_mm_fini(struct vbox_private *vbox)
> > -{
> > -	arch_phys_wc_del(vbox->fb_mtrr);
> > -}
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch