On Mon, Apr 6, 2020 at 7:27 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>
> Hi
>
> Am 03.04.20 um 15:57 schrieb Daniel Vetter:
> > We use the baseclass pattern here, so lets to the proper (and more
> > typesafe) upcasting.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> > ---
> > drivers/gpu/drm/vboxvideo/vbox_drv.c | 1 -
> > drivers/gpu/drm/vboxvideo/vbox_drv.h | 1 +
> > drivers/gpu/drm/vboxvideo/vbox_irq.c | 2 +-
> > drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +++++-----
> > 4 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > index be0600b22cf5..d34cddd809fd 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > @@ -52,7 +52,6 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > return PTR_ERR(vbox);
> >
> > vbox->ddev.pdev = pdev;
> > - vbox->ddev.dev_private = vbox;
> > pci_set_drvdata(pdev, vbox);
> > mutex_init(&vbox->hw_mutex);
> >
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > index 87421903816c..ac7c2effc46f 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > @@ -127,6 +127,7 @@ struct vbox_encoder {
> > #define to_vbox_crtc(x) container_of(x, struct vbox_crtc, base)
> > #define to_vbox_connector(x) container_of(x, struct vbox_connector, base)
> > #define to_vbox_encoder(x) container_of(x, struct vbox_encoder, base)
> > +#define to_vbox_dev(x) container_of(x, struct vbox_private, ddev)
>
> I suggest ot call this macro to to_vbox_device(). At some point, we
> should rename struct vbox_private to struct vbox_device for consistency
> among drivers. The new macro's name would then fit.
So I've seen naming conventions around this with a _dev suffix, a _drm
suffix, a _priv suffix and no suffix (since it's the top level object
you kinda can justify that too). I admit the choice I went with was
occasionally a bit random, but that's mostly because there's not much
consistency here at all. This applies both to the upcast macro and the
struct itself.
iow I'm not sure this bikeshed is worth repainting, current status is
rather multicolor already ... It's also an enormous amounts of churn
to repaint this stuff already (just git grep dev_private --
drivers/gpu/drm), so I'm not super enthusiastic about adding more
churn on top ...
Still feel strongly about this one and the others you've brought up?
-Daniel
> For the overall patch:
>
> Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>
> Best regards
> Thomas
>
> >
> > bool vbox_check_supported(u16 id);
> > int vbox_hw_init(struct vbox_private *vbox);
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_irq.c b/drivers/gpu/drm/vboxvideo/vbox_irq.c
> > index 16a1e29f5292..631657fa554f 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_irq.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_irq.c
> > @@ -34,7 +34,7 @@ void vbox_report_hotplug(struct vbox_private *vbox)
> > irqreturn_t vbox_irq_handler(int irq, void *arg)
> > {
> > struct drm_device *dev = (struct drm_device *)arg;
> > - struct vbox_private *vbox = (struct vbox_private *)dev->dev_private;
> > + struct vbox_private *vbox = to_vbox_dev(dev);
> > u32 host_flags = vbox_get_flags(vbox);
> >
> > if (!(host_flags & HGSMIHOSTFLAGS_IRQ))
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > index 0883a435e62b..d9a5af62af89 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> > @@ -36,7 +36,7 @@ static void vbox_do_modeset(struct drm_crtc *crtc)
> > u16 flags;
> > s32 x_offset, y_offset;
> >
> > - vbox = crtc->dev->dev_private;
> > + vbox = to_vbox_dev(crtc->dev);
> > width = vbox_crtc->width ? vbox_crtc->width : 640;
> > height = vbox_crtc->height ? vbox_crtc->height : 480;
> > bpp = fb ? fb->format->cpp[0] * 8 : 32;
> > @@ -77,7 +77,7 @@ static void vbox_do_modeset(struct drm_crtc *crtc)
> > static int vbox_set_view(struct drm_crtc *crtc)
> > {
> > struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> > - struct vbox_private *vbox = crtc->dev->dev_private;
> > + struct vbox_private *vbox = to_vbox_dev(crtc->dev);
> > struct vbva_infoview *p;
> >
> > /*
> > @@ -174,7 +174,7 @@ static void vbox_crtc_set_base_and_mode(struct drm_crtc *crtc,
> > int x, int y)
> > {
> > struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(fb->obj[0]);
> > - struct vbox_private *vbox = crtc->dev->dev_private;
> > + struct vbox_private *vbox = to_vbox_dev(crtc->dev);
> > struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> > bool needs_modeset = drm_atomic_crtc_needs_modeset(crtc->state);
> >
> > @@ -272,7 +272,7 @@ static void vbox_primary_atomic_update(struct drm_plane *plane,
> > {
> > struct drm_crtc *crtc = plane->state->crtc;
> > struct drm_framebuffer *fb = plane->state->fb;
> > - struct vbox_private *vbox = fb->dev->dev_private;
> > + struct vbox_private *vbox = to_vbox_dev(fb->dev);
> > struct drm_mode_rect *clips;
> > uint32_t num_clips, i;
> >
> > @@ -704,7 +704,7 @@ static int vbox_get_modes(struct drm_connector *connector)
> > int preferred_width, preferred_height;
> >
> > vbox_connector = to_vbox_connector(connector);
> > - vbox = connector->dev->dev_private;
> > + vbox = to_vbox_dev(connector->dev);
> >
> > hgsmi_report_flags_location(vbox->guest_pool, GUEST_HEAP_OFFSET(vbox) +
> > HOST_FLAGS_OFFSET);
> >
>
> --
> 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
