| Message ID | 20200403135828.2542770-41-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | devm_drm_dev_alloc, no more drmm_add_final_kfree | expand |
On Fri, Apr 3, 2020 at 6:59 AM Daniel Vetter <daniel.vetter@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@intel.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: "Noralf Trønnes" <noralf@tronnes.org> > Cc: Sam Ravnborg <sam@ravnborg.org> Acked-by: Eric Anholt <eric@anholt.net>
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@intel.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: "Noralf Trønnes" <noralf@tronnes.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Eric Anholt <eric@anholt.net> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: virtualization@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)); >
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@intel.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: "Noralf Trønnes" <noralf@tronnes.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Eric Anholt <eric@anholt.net> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: virtualization@lists.linux-foundation.org Acked-by: Sam Ravnborg <sam@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@intel.com> > > Cc: Dave Airlie <airlied@redhat.com> > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: "Noralf Trønnes" <noralf@tronnes.org> > > Cc: Sam Ravnborg <sam@ravnborg.org> > > Cc: Eric Anholt <eric@anholt.net> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: virtualization@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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));
Upcasting using a container_of macro is more typesafe, faster and easier for the compiler to optimize. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Dave Airlie <airlied@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: "Noralf Trønnes" <noralf@tronnes.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Eric Anholt <eric@anholt.net> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: virtualization@lists.linux-foundation.org --- drivers/gpu/drm/cirrus/cirrus.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)