Re: [PATCH 08/44] drm/vboxvideo: Stop using drm_device->dev_private

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 07.04.20 um 09:24 schrieb Daniel Vetter:
> 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?

I have no strong feelings about it. :) But since you're introducing the
macro, I thought that naming it to_vbox_device() would be consisted
within the driver.

Best regards
Thomas

> -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
>>
> 
> 

-- 
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

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux