Daniel Vetter
2020-Apr-03 13:58 UTC
[PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
Upcasting using a container_of macro is more typesafe, faster and easier for the compiler to optimize. Signed-off-by: Daniel Vetter <daniel.vetter at intel.com> Cc: Dave Airlie <airlied at redhat.com> Cc: Gerd Hoffmann <kraxel at redhat.com> Cc: Daniel Vetter <daniel.vetter at ffwll.ch> Cc: "Noralf Tr?nnes" <noralf at tronnes.org> Cc: Sam Ravnborg <sam at ravnborg.org> Cc: Eric Anholt <eric at anholt.net> Cc: Thomas Zimmermann <tzimmermann at suse.de> Cc: virtualization at lists.linux-foundation.org --- drivers/gpu/drm/cirrus/cirrus.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c index 4b65637147ba..744a8e337e41 100644 --- a/drivers/gpu/drm/cirrus/cirrus.c +++ b/drivers/gpu/drm/cirrus/cirrus.c @@ -59,6 +59,8 @@ struct cirrus_device { void __iomem *mmio; }; +#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev) + /* ------------------------------------------------------------------ */ /* * The meat of this driver. The core passes us a mode and we have to program @@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus, static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, struct drm_rect *rect) { - struct cirrus_device *cirrus = fb->dev->dev_private; + struct cirrus_device *cirrus = to_cirrus(fb->dev); void *vmap; int idx, ret; @@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state, struct drm_plane_state *plane_state) { - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); cirrus_fb_blit_fullscreen(plane_state->fb); @@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_plane_state *old_state) { - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); struct drm_plane_state *state = pipe->plane.state; struct drm_crtc *crtc = &pipe->crtc; struct drm_rect rect; @@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev, return PTR_ERR(cirrus); dev = &cirrus->dev; - dev->dev_private = cirrus; cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0), pci_resource_len(pdev, 0)); -- 2.25.1
Eric Anholt
2020-Apr-03 17:40 UTC
[PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
On Fri, Apr 3, 2020 at 6:59 AM Daniel Vetter <daniel.vetter at ffwll.ch> wrote:> > Upcasting using a container_of macro is more typesafe, faster and > easier for the compiler to optimize. > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com> > Cc: Dave Airlie <airlied at redhat.com> > Cc: Gerd Hoffmann <kraxel at redhat.com> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch> > Cc: "Noralf Tr?nnes" <noralf at tronnes.org> > Cc: Sam Ravnborg <sam at ravnborg.org>Acked-by: Eric Anholt <eric at anholt.net>
Thomas Zimmermann
2020-Apr-06 11:58 UTC
[PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
Am 03.04.20 um 15:58 schrieb Daniel Vetter:> Upcasting using a container_of macro is more typesafe, faster and > easier for the compiler to optimize. > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com> > Cc: Dave Airlie <airlied at redhat.com> > Cc: Gerd Hoffmann <kraxel at redhat.com> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch> > Cc: "Noralf Tr?nnes" <noralf at tronnes.org> > Cc: Sam Ravnborg <sam at ravnborg.org> > Cc: Eric Anholt <eric at anholt.net> > Cc: Thomas Zimmermann <tzimmermann at suse.de> > Cc: virtualization at lists.linux-foundation.org > --- > drivers/gpu/drm/cirrus/cirrus.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c > index 4b65637147ba..744a8e337e41 100644 > --- a/drivers/gpu/drm/cirrus/cirrus.c > +++ b/drivers/gpu/drm/cirrus/cirrus.c > @@ -59,6 +59,8 @@ struct cirrus_device { > void __iomem *mmio; > }; > > +#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev) > +Maybe to_cirrus_device() ? I had the same comment for vbox and I think it applies to all patches. Best regards Thomas> /* ------------------------------------------------------------------ */ > /* > * The meat of this driver. The core passes us a mode and we have to program > @@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus, > static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > struct drm_rect *rect) > { > - struct cirrus_device *cirrus = fb->dev->dev_private; > + struct cirrus_device *cirrus = to_cirrus(fb->dev); > void *vmap; > int idx, ret; > > @@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > struct drm_crtc_state *crtc_state, > struct drm_plane_state *plane_state) > { > - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); > > cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); > cirrus_fb_blit_fullscreen(plane_state->fb); > @@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, > struct drm_plane_state *old_state) > { > - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); > struct drm_plane_state *state = pipe->plane.state; > struct drm_crtc *crtc = &pipe->crtc; > struct drm_rect rect; > @@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev, > return PTR_ERR(cirrus); > > dev = &cirrus->dev; > - dev->dev_private = cirrus; > > cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0), > pci_resource_len(pdev, 0)); >-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 N?rnberg, Germany (HRB 36809, AG N?rnberg) Gesch?ftsf?hrer: Felix Imend?rffer -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: OpenPGP digital signature URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20200406/920d5880/attachment.sig>
Sam Ravnborg
2020-Apr-08 08:01 UTC
[PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
On Fri, Apr 03, 2020 at 03:58:24PM +0200, Daniel Vetter wrote:> Upcasting using a container_of macro is more typesafe, faster and > easier for the compiler to optimize. > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com> > Cc: Dave Airlie <airlied at redhat.com> > Cc: Gerd Hoffmann <kraxel at redhat.com> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch> > Cc: "Noralf Tr?nnes" <noralf at tronnes.org> > Cc: Sam Ravnborg <sam at ravnborg.org> > Cc: Eric Anholt <eric at anholt.net> > Cc: Thomas Zimmermann <tzimmermann at suse.de> > Cc: virtualization at lists.linux-foundation.orgAcked-by: Sam Ravnborg <sam at ravnborg.org>> --- > drivers/gpu/drm/cirrus/cirrus.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c > index 4b65637147ba..744a8e337e41 100644 > --- a/drivers/gpu/drm/cirrus/cirrus.c > +++ b/drivers/gpu/drm/cirrus/cirrus.c > @@ -59,6 +59,8 @@ struct cirrus_device { > void __iomem *mmio; > }; > > +#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev) > + > /* ------------------------------------------------------------------ */ > /* > * The meat of this driver. The core passes us a mode and we have to program > @@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus, > static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > struct drm_rect *rect) > { > - struct cirrus_device *cirrus = fb->dev->dev_private; > + struct cirrus_device *cirrus = to_cirrus(fb->dev); > void *vmap; > int idx, ret; > > @@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > struct drm_crtc_state *crtc_state, > struct drm_plane_state *plane_state) > { > - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); > > cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); > cirrus_fb_blit_fullscreen(plane_state->fb); > @@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, > struct drm_plane_state *old_state) > { > - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); > struct drm_plane_state *state = pipe->plane.state; > struct drm_crtc *crtc = &pipe->crtc; > struct drm_rect rect; > @@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev, > return PTR_ERR(cirrus); > > dev = &cirrus->dev; > - dev->dev_private = cirrus; > > cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0), > pci_resource_len(pdev, 0)); > -- > 2.25.1 > > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sam Ravnborg
2020-Apr-08 08:04 UTC
[PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
Hi Thomas. On Mon, Apr 06, 2020 at 01:58:54PM +0200, Thomas Zimmermann wrote:> > > Am 03.04.20 um 15:58 schrieb Daniel Vetter: > > Upcasting using a container_of macro is more typesafe, faster and > > easier for the compiler to optimize. > > > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com> > > Cc: Dave Airlie <airlied at redhat.com> > > Cc: Gerd Hoffmann <kraxel at redhat.com> > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch> > > Cc: "Noralf Tr?nnes" <noralf at tronnes.org> > > Cc: Sam Ravnborg <sam at ravnborg.org> > > Cc: Eric Anholt <eric at anholt.net> > > Cc: Thomas Zimmermann <tzimmermann at suse.de> > > Cc: virtualization at lists.linux-foundation.org > > --- > > drivers/gpu/drm/cirrus/cirrus.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c > > index 4b65637147ba..744a8e337e41 100644 > > --- a/drivers/gpu/drm/cirrus/cirrus.c > > +++ b/drivers/gpu/drm/cirrus/cirrus.c > > @@ -59,6 +59,8 @@ struct cirrus_device { > > void __iomem *mmio; > > }; > > > > +#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev) > > + > > Maybe to_cirrus_device() ? I had the same comment for vbox and I think > it applies to all patches.The variable name is consistently using the name "cirrus" - so my personal preference is to_cirrus(). Also IMO struct cirrus_device is misnamed. It is more than a device. Sam> > Best regards > Thomas > > > /* ------------------------------------------------------------------ */ > > /* > > * The meat of this driver. The core passes us a mode and we have to program > > @@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus, > > static int cirrus_fb_blit_rect(struct drm_framebuffer *fb, > > struct drm_rect *rect) > > { > > - struct cirrus_device *cirrus = fb->dev->dev_private; > > + struct cirrus_device *cirrus = to_cirrus(fb->dev); > > void *vmap; > > int idx, ret; > > > > @@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > > struct drm_crtc_state *crtc_state, > > struct drm_plane_state *plane_state) > > { > > - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > > + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); > > > > cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb); > > cirrus_fb_blit_fullscreen(plane_state->fb); > > @@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe, > > static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe, > > struct drm_plane_state *old_state) > > { > > - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private; > > + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev); > > struct drm_plane_state *state = pipe->plane.state; > > struct drm_crtc *crtc = &pipe->crtc; > > struct drm_rect rect; > > @@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev, > > return PTR_ERR(cirrus); > > > > dev = &cirrus->dev; > > - dev->dev_private = cirrus; > > > > cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0), > > pci_resource_len(pdev, 0)); > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 N?rnberg, Germany > (HRB 36809, AG N?rnberg) > Gesch?ftsf?hrer: Felix Imend?rffer >> _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Seemingly Similar Threads
- [PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
- [PATCH 00/17] cirrus: Modernize the cirrus driver
- [PATCH 36/59] drm/cirrus: Don't use drm_device->dev_private
- [PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
- [PATCH] drm/cirrus: rewrite and modernize driver.