Re: [PATCH] drm: Rename drm_bridge->dev to drm
On Tue, Dec 10, 2019 at 01:28:42PM +, Mihail Atanassov wrote: > On Tuesday, 10 December 2019 10:12:50 GMT Daniel Vetter wrote: > > On Fri, Dec 06, 2019 at 12:59:04PM +0100, Thomas Zimmermann wrote: > > > Hi > > > > > > Am 06.12.19 um 12:25 schrieb Mihail Atanassov: > > > > Hallo Thomas, > > > > > > > > On Thursday, 5 December 2019 18:20:06 GMT Thomas Zimmermann wrote: > > > >> Hi > > > >> > > > >> Am 05.12.19 um 17:30 schrieb Mihail Atanassov: > > > >>> The 'dev' name causes some confusion with 'struct device' [1][2], so > > > >>> use > > > >>> 'drm' instead since this seems to be the prevalent name for 'struct > > > >>> drm_device' members. > > > >>> > > > >>> [1] https://patchwork.freedesktop.org/patch/342472/?series=70039=1 > > > >>> [2] https://patchwork.freedesktop.org/patch/343643/?series=70432=1 > > > >> > > > >> I read through the provided links, but can't see why is it called > > > >> 'drm'. > > > >> That sounds like a reference to a DRM driver structure to me. > > > > > > > > There are about 550 hits on 'struct drm_device *drm', so I gathered that > > > > it's the most common alternative to just naming it 'dev' (at about 4.5k > > > > hits in the codebase). There's also 'ddev', 'drm_dev', 'drmdev' with > > > > few hits. > > > > > > > >> > > > >> What about attached_dev or consumer_dev or encoder_dev? > > > > > > > > Those would be more descriptive, if you think it's worth sidestepping > > > > the above convention a bit. I don't mind either way. > > > > > > Well, I don't have a say on these things, but it's worth considering a > > > more descriptive name IMHO. > > > > > > I also wonder why that field is there in the first place. Invoking > > > drm_bridge_attach() sets the encoder and its dev field for the bridge. > > > [1] Could the dev field be removed and all users refer to encoder->dev > > > instead? If so, it seems like the better way to go. > > > > That sounds like a pretty neat idea (if possible) ... > > -Daniel > > > > OK, I'll poke at it a bit and see what falls out. I'm guessing we don't > particularly care about the extra pointer being dereferenced in driver code? > The vast majority of the uses are in attach/detach logic so fairly benign but > we do have a few in IRQs. Modeset code is "readability over speed, any time" except if someone can show in a real world use case that it actually matters somewhere. Spoiler: In 10 years it never did. So go ahead without concerns, we have a lot more pointer chasing going on here than just that :-) -Daniel > > > > > > > Best regards > > > Thomas > > > > > > [1] > > > https://elixir.bootlin.com/linux/v5.4.2/source/drivers/gpu/drm/drm_bridge.c#L128 > > > > > > > > > > >> > > > >> Best regards > > > >> Thomas > > > >> > > > >>> > > > >>> Cc: Daniel Vetter > > > >>> Cc: Laurent Pinchart > > > >>> Signed-off-by: Mihail Atanassov > > > >>> --- > > > >>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > > > >>> drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +- > > > >>> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 +- > > > >>> drivers/gpu/drm/bridge/cdns-dsi.c| 2 +- > > > >>> drivers/gpu/drm/bridge/dumb-vga-dac.c| 2 +- > > > >>> .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- > > > >>> drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- > > > >>> drivers/gpu/drm/bridge/panel.c | 2 +- > > > >>> drivers/gpu/drm/bridge/parade-ps8622.c | 2 +- > > > >>> drivers/gpu/drm/bridge/sii902x.c | 6 +++--- > > > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 6 +++--- > > > >>> drivers/gpu/drm/bridge/tc358764.c| 4 ++-- > > > >>> drivers/gpu/drm/bridge/tc358767.c| 6 +++--- > > > >>> drivers/gpu/drm/bridge/ti-sn65dsi86.c| 2 +- > > > >>> drivers/gpu/drm/bridge/ti-tfp410.c | 6 +++--- > > > >>> drivers/gpu/drm/drm_bridge.c | 12 > > > >>> ++-- > > > >>> drivers/gpu/drm/i2c/tda998x_drv.c| 2 +- > > > >>> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > > > >>> drivers/gpu/drm/msm/edp/edp_bridge.c | 2 +- > > > >>> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++-- > > > >>> drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +- > > > >>> include/drm/drm_bridge.h | 4 ++-- > > > >>> 22 files changed, 38 insertions(+), 38 deletions(-) > > > >>> > > > >>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > >>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > >>> index 9e13e466e72c..db7d01cb0923 100644 > > > >>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > >>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > >>> @@ -863,7 +863,7 @@ static int adv7511_bridge_attach(struct > > > >>> drm_bridge *bridge) > > > >>>
Re: [PATCH] drm: Rename drm_bridge->dev to drm
On Tuesday, 10 December 2019 10:12:50 GMT Daniel Vetter wrote:
> On Fri, Dec 06, 2019 at 12:59:04PM +0100, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 06.12.19 um 12:25 schrieb Mihail Atanassov:
> > > Hallo Thomas,
> > >
> > > On Thursday, 5 December 2019 18:20:06 GMT Thomas Zimmermann wrote:
> > >> Hi
> > >>
> > >> Am 05.12.19 um 17:30 schrieb Mihail Atanassov:
> > >>> The 'dev' name causes some confusion with 'struct device' [1][2], so use
> > >>> 'drm' instead since this seems to be the prevalent name for 'struct
> > >>> drm_device' members.
> > >>>
> > >>> [1] https://patchwork.freedesktop.org/patch/342472/?series=70039=1
> > >>> [2] https://patchwork.freedesktop.org/patch/343643/?series=70432=1
> > >>
> > >> I read through the provided links, but can't see why is it called 'drm'.
> > >> That sounds like a reference to a DRM driver structure to me.
> > >
> > > There are about 550 hits on 'struct drm_device *drm', so I gathered that
> > > it's the most common alternative to just naming it 'dev' (at about 4.5k
> > > hits in the codebase). There's also 'ddev', 'drm_dev', 'drmdev' with
> > > few hits.
> > >
> > >>
> > >> What about attached_dev or consumer_dev or encoder_dev?
> > >
> > > Those would be more descriptive, if you think it's worth sidestepping
> > > the above convention a bit. I don't mind either way.
> >
> > Well, I don't have a say on these things, but it's worth considering a
> > more descriptive name IMHO.
> >
> > I also wonder why that field is there in the first place. Invoking
> > drm_bridge_attach() sets the encoder and its dev field for the bridge.
> > [1] Could the dev field be removed and all users refer to encoder->dev
> > instead? If so, it seems like the better way to go.
>
> That sounds like a pretty neat idea (if possible) ...
> -Daniel
>
OK, I'll poke at it a bit and see what falls out. I'm guessing we don't
particularly care about the extra pointer being dereferenced in driver code?
The vast majority of the uses are in attach/detach logic so fairly benign but
we do have a few in IRQs.
> >
> > Best regards
> > Thomas
> >
> > [1]
> > https://elixir.bootlin.com/linux/v5.4.2/source/drivers/gpu/drm/drm_bridge.c#L128
> >
> > >
> > >>
> > >> Best regards
> > >> Thomas
> > >>
> > >>>
> > >>> Cc: Daniel Vetter
> > >>> Cc: Laurent Pinchart
> > >>> Signed-off-by: Mihail Atanassov
> > >>> ---
> > >>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
> > >>> drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +-
> > >>> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 +-
> > >>> drivers/gpu/drm/bridge/cdns-dsi.c| 2 +-
> > >>> drivers/gpu/drm/bridge/dumb-vga-dac.c| 2 +-
> > >>> .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +-
> > >>> drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> > >>> drivers/gpu/drm/bridge/panel.c | 2 +-
> > >>> drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> > >>> drivers/gpu/drm/bridge/sii902x.c | 6 +++---
> > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 6 +++---
> > >>> drivers/gpu/drm/bridge/tc358764.c| 4 ++--
> > >>> drivers/gpu/drm/bridge/tc358767.c| 6 +++---
> > >>> drivers/gpu/drm/bridge/ti-sn65dsi86.c| 2 +-
> > >>> drivers/gpu/drm/bridge/ti-tfp410.c | 6 +++---
> > >>> drivers/gpu/drm/drm_bridge.c | 12 ++--
> > >>> drivers/gpu/drm/i2c/tda998x_drv.c| 2 +-
> > >>> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> > >>> drivers/gpu/drm/msm/edp/edp_bridge.c | 2 +-
> > >>> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
> > >>> drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> > >>> include/drm/drm_bridge.h | 4 ++--
> > >>> 22 files changed, 38 insertions(+), 38 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > >>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > >>> index 9e13e466e72c..db7d01cb0923 100644
> > >>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > >>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > >>> @@ -863,7 +863,7 @@ static int adv7511_bridge_attach(struct drm_bridge
> > >>> *bridge)
> > >>> adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> > >>> DRM_CONNECTOR_POLL_DISCONNECT;
> > >>>
> > >>> - ret = drm_connector_init(bridge->dev, >connector,
> > >>> + ret = drm_connector_init(bridge->drm, >connector,
> > >>> _connector_funcs,
> > >>> DRM_MODE_CONNECTOR_HDMIA);
> > >>> if (ret) {
> > >>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > >>> b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > >>> index
Re: [PATCH] drm: Rename drm_bridge->dev to drm
On Fri, Dec 06, 2019 at 12:59:04PM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 06.12.19 um 12:25 schrieb Mihail Atanassov:
> > Hallo Thomas,
> >
> > On Thursday, 5 December 2019 18:20:06 GMT Thomas Zimmermann wrote:
> >> Hi
> >>
> >> Am 05.12.19 um 17:30 schrieb Mihail Atanassov:
> >>> The 'dev' name causes some confusion with 'struct device' [1][2], so use
> >>> 'drm' instead since this seems to be the prevalent name for 'struct
> >>> drm_device' members.
> >>>
> >>> [1] https://patchwork.freedesktop.org/patch/342472/?series=70039=1
> >>> [2] https://patchwork.freedesktop.org/patch/343643/?series=70432=1
> >>
> >> I read through the provided links, but can't see why is it called 'drm'.
> >> That sounds like a reference to a DRM driver structure to me.
> >
> > There are about 550 hits on 'struct drm_device *drm', so I gathered that
> > it's the most common alternative to just naming it 'dev' (at about 4.5k
> > hits in the codebase). There's also 'ddev', 'drm_dev', 'drmdev' with
> > few hits.
> >
> >>
> >> What about attached_dev or consumer_dev or encoder_dev?
> >
> > Those would be more descriptive, if you think it's worth sidestepping
> > the above convention a bit. I don't mind either way.
>
> Well, I don't have a say on these things, but it's worth considering a
> more descriptive name IMHO.
>
> I also wonder why that field is there in the first place. Invoking
> drm_bridge_attach() sets the encoder and its dev field for the bridge.
> [1] Could the dev field be removed and all users refer to encoder->dev
> instead? If so, it seems like the better way to go.
That sounds like a pretty neat idea (if possible) ...
-Daniel
>
> Best regards
> Thomas
>
> [1]
> https://elixir.bootlin.com/linux/v5.4.2/source/drivers/gpu/drm/drm_bridge.c#L128
>
> >
> >>
> >> Best regards
> >> Thomas
> >>
> >>>
> >>> Cc: Daniel Vetter
> >>> Cc: Laurent Pinchart
> >>> Signed-off-by: Mihail Atanassov
> >>> ---
> >>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
> >>> drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +-
> >>> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 +-
> >>> drivers/gpu/drm/bridge/cdns-dsi.c| 2 +-
> >>> drivers/gpu/drm/bridge/dumb-vga-dac.c| 2 +-
> >>> .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +-
> >>> drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> >>> drivers/gpu/drm/bridge/panel.c | 2 +-
> >>> drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> >>> drivers/gpu/drm/bridge/sii902x.c | 6 +++---
> >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 6 +++---
> >>> drivers/gpu/drm/bridge/tc358764.c| 4 ++--
> >>> drivers/gpu/drm/bridge/tc358767.c| 6 +++---
> >>> drivers/gpu/drm/bridge/ti-sn65dsi86.c| 2 +-
> >>> drivers/gpu/drm/bridge/ti-tfp410.c | 6 +++---
> >>> drivers/gpu/drm/drm_bridge.c | 12 ++--
> >>> drivers/gpu/drm/i2c/tda998x_drv.c| 2 +-
> >>> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> >>> drivers/gpu/drm/msm/edp/edp_bridge.c | 2 +-
> >>> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
> >>> drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> >>> include/drm/drm_bridge.h | 4 ++--
> >>> 22 files changed, 38 insertions(+), 38 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> >>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> >>> index 9e13e466e72c..db7d01cb0923 100644
> >>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> >>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> >>> @@ -863,7 +863,7 @@ static int adv7511_bridge_attach(struct drm_bridge
> >>> *bridge)
> >>> adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> >>> DRM_CONNECTOR_POLL_DISCONNECT;
> >>>
> >>> - ret = drm_connector_init(bridge->dev, >connector,
> >>> + ret = drm_connector_init(bridge->drm, >connector,
> >>>_connector_funcs,
> >>>DRM_MODE_CONNECTOR_HDMIA);
> >>> if (ret) {
> >>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >>> b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >>> index b4f3a923a52a..0e3508aeaa6c 100644
> >>> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >>> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >>> @@ -541,7 +541,7 @@ static int anx6345_bridge_attach(struct drm_bridge
> >>> *bridge)
> >>> return err;
> >>> }
> >>>
> >>> - err = drm_connector_init(bridge->dev, >connector,
> >>> + err = drm_connector_init(bridge->drm, >connector,
> >>>_connector_funcs,
> >>>DRM_MODE_CONNECTOR_eDP);
> >>> if (err) {
> >>> diff --git
Re: [PATCH] drm: Rename drm_bridge->dev to drm
Hi
Am 06.12.19 um 12:25 schrieb Mihail Atanassov:
> Hallo Thomas,
>
> On Thursday, 5 December 2019 18:20:06 GMT Thomas Zimmermann wrote:
>> Hi
>>
>> Am 05.12.19 um 17:30 schrieb Mihail Atanassov:
>>> The 'dev' name causes some confusion with 'struct device' [1][2], so use
>>> 'drm' instead since this seems to be the prevalent name for 'struct
>>> drm_device' members.
>>>
>>> [1] https://patchwork.freedesktop.org/patch/342472/?series=70039=1
>>> [2] https://patchwork.freedesktop.org/patch/343643/?series=70432=1
>>
>> I read through the provided links, but can't see why is it called 'drm'.
>> That sounds like a reference to a DRM driver structure to me.
>
> There are about 550 hits on 'struct drm_device *drm', so I gathered that
> it's the most common alternative to just naming it 'dev' (at about 4.5k
> hits in the codebase). There's also 'ddev', 'drm_dev', 'drmdev' with
> few hits.
>
>>
>> What about attached_dev or consumer_dev or encoder_dev?
>
> Those would be more descriptive, if you think it's worth sidestepping
> the above convention a bit. I don't mind either way.
Well, I don't have a say on these things, but it's worth considering a
more descriptive name IMHO.
I also wonder why that field is there in the first place. Invoking
drm_bridge_attach() sets the encoder and its dev field for the bridge.
[1] Could the dev field be removed and all users refer to encoder->dev
instead? If so, it seems like the better way to go.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v5.4.2/source/drivers/gpu/drm/drm_bridge.c#L128
>
>>
>> Best regards
>> Thomas
>>
>>>
>>> Cc: Daniel Vetter
>>> Cc: Laurent Pinchart
>>> Signed-off-by: Mihail Atanassov
>>> ---
>>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
>>> drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +-
>>> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 +-
>>> drivers/gpu/drm/bridge/cdns-dsi.c| 2 +-
>>> drivers/gpu/drm/bridge/dumb-vga-dac.c| 2 +-
>>> .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +-
>>> drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
>>> drivers/gpu/drm/bridge/panel.c | 2 +-
>>> drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
>>> drivers/gpu/drm/bridge/sii902x.c | 6 +++---
>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 6 +++---
>>> drivers/gpu/drm/bridge/tc358764.c| 4 ++--
>>> drivers/gpu/drm/bridge/tc358767.c| 6 +++---
>>> drivers/gpu/drm/bridge/ti-sn65dsi86.c| 2 +-
>>> drivers/gpu/drm/bridge/ti-tfp410.c | 6 +++---
>>> drivers/gpu/drm/drm_bridge.c | 12 ++--
>>> drivers/gpu/drm/i2c/tda998x_drv.c| 2 +-
>>> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
>>> drivers/gpu/drm/msm/edp/edp_bridge.c | 2 +-
>>> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
>>> drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
>>> include/drm/drm_bridge.h | 4 ++--
>>> 22 files changed, 38 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> index 9e13e466e72c..db7d01cb0923 100644
>>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> @@ -863,7 +863,7 @@ static int adv7511_bridge_attach(struct drm_bridge
>>> *bridge)
>>> adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
>>> DRM_CONNECTOR_POLL_DISCONNECT;
>>>
>>> - ret = drm_connector_init(bridge->dev, >connector,
>>> + ret = drm_connector_init(bridge->drm, >connector,
>>> _connector_funcs,
>>> DRM_MODE_CONNECTOR_HDMIA);
>>> if (ret) {
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
>>> b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
>>> index b4f3a923a52a..0e3508aeaa6c 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
>>> @@ -541,7 +541,7 @@ static int anx6345_bridge_attach(struct drm_bridge
>>> *bridge)
>>> return err;
>>> }
>>>
>>> - err = drm_connector_init(bridge->dev, >connector,
>>> + err = drm_connector_init(bridge->drm, >connector,
>>> _connector_funcs,
>>> DRM_MODE_CONNECTOR_eDP);
>>> if (err) {
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
>>> b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
>>> index 41867be03751..d5722bc28933 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
>>> @@ -908,7 +908,7 @@ static int
Re: [PATCH] drm: Rename drm_bridge->dev to drm
Hallo Thomas,
On Thursday, 5 December 2019 18:20:06 GMT Thomas Zimmermann wrote:
> Hi
>
> Am 05.12.19 um 17:30 schrieb Mihail Atanassov:
> > The 'dev' name causes some confusion with 'struct device' [1][2], so use
> > 'drm' instead since this seems to be the prevalent name for 'struct
> > drm_device' members.
> >
> > [1] https://patchwork.freedesktop.org/patch/342472/?series=70039=1
> > [2] https://patchwork.freedesktop.org/patch/343643/?series=70432=1
>
> I read through the provided links, but can't see why is it called 'drm'.
> That sounds like a reference to a DRM driver structure to me.
There are about 550 hits on 'struct drm_device *drm', so I gathered that
it's the most common alternative to just naming it 'dev' (at about 4.5k
hits in the codebase). There's also 'ddev', 'drm_dev', 'drmdev' with
few hits.
>
> What about attached_dev or consumer_dev or encoder_dev?
Those would be more descriptive, if you think it's worth sidestepping
the above convention a bit. I don't mind either way.
>
> Best regards
> Thomas
>
> >
> > Cc: Daniel Vetter
> > Cc: Laurent Pinchart
> > Signed-off-by: Mihail Atanassov
> > ---
> > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
> > drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +-
> > drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 +-
> > drivers/gpu/drm/bridge/cdns-dsi.c| 2 +-
> > drivers/gpu/drm/bridge/dumb-vga-dac.c| 2 +-
> > .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +-
> > drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> > drivers/gpu/drm/bridge/panel.c | 2 +-
> > drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> > drivers/gpu/drm/bridge/sii902x.c | 6 +++---
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 6 +++---
> > drivers/gpu/drm/bridge/tc358764.c| 4 ++--
> > drivers/gpu/drm/bridge/tc358767.c| 6 +++---
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c| 2 +-
> > drivers/gpu/drm/bridge/ti-tfp410.c | 6 +++---
> > drivers/gpu/drm/drm_bridge.c | 12 ++--
> > drivers/gpu/drm/i2c/tda998x_drv.c| 2 +-
> > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> > drivers/gpu/drm/msm/edp/edp_bridge.c | 2 +-
> > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
> > drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> > include/drm/drm_bridge.h | 4 ++--
> > 22 files changed, 38 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > index 9e13e466e72c..db7d01cb0923 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > @@ -863,7 +863,7 @@ static int adv7511_bridge_attach(struct drm_bridge
> > *bridge)
> > adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> > DRM_CONNECTOR_POLL_DISCONNECT;
> >
> > - ret = drm_connector_init(bridge->dev, >connector,
> > + ret = drm_connector_init(bridge->drm, >connector,
> > _connector_funcs,
> > DRM_MODE_CONNECTOR_HDMIA);
> > if (ret) {
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > index b4f3a923a52a..0e3508aeaa6c 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> > @@ -541,7 +541,7 @@ static int anx6345_bridge_attach(struct drm_bridge
> > *bridge)
> > return err;
> > }
> >
> > - err = drm_connector_init(bridge->dev, >connector,
> > + err = drm_connector_init(bridge->drm, >connector,
> > _connector_funcs,
> > DRM_MODE_CONNECTOR_eDP);
> > if (err) {
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > index 41867be03751..d5722bc28933 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> > @@ -908,7 +908,7 @@ static int anx78xx_bridge_attach(struct drm_bridge
> > *bridge)
> > return err;
> > }
> >
> > - err = drm_connector_init(bridge->dev, >connector,
> > + err = drm_connector_init(bridge->drm, >connector,
> > _connector_funcs,
> > DRM_MODE_CONNECTOR_DisplayPort);
> > if (err) {
> > diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
> > b/drivers/gpu/drm/bridge/cdns-dsi.c
> > index 3a5bd4e7fd1e..f6d7e97de66e 100644
> > --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> > +++
Re: [PATCH] drm: Rename drm_bridge->dev to drm
Hi Mihail. On Thu, Dec 05, 2019 at 04:30:45PM +, Mihail Atanassov wrote: > The 'dev' name causes some confusion with 'struct device' [1][2], so use > 'drm' instead since this seems to be the prevalent name for 'struct > drm_device' members. Thanks for doing this - it helps readability. checkpatch complained: -:107: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #107: FILE: drivers/gpu/drm/bridge/nxp-ptn3460.c:251: + ret = drm_connector_init(bridge->drm, _bridge->connector, _connector_funcs, DRM_MODE_CONNECTOR_LVDS); -:133: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #133: FILE: drivers/gpu/drm/bridge/parade-ps8622.c:491: + ret = drm_connector_init(bridge->drm, >connector, _connector_funcs, DRM_MODE_CONNECTOR_LVDS); But this seems unrelated to your changes - so should be ignored. Browsed the patch and throw it after my build check script. All looked good. Acked-by: Sam Ravnborg Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Rename drm_bridge->dev to drm
Hi
Am 05.12.19 um 17:30 schrieb Mihail Atanassov:
> The 'dev' name causes some confusion with 'struct device' [1][2], so use
> 'drm' instead since this seems to be the prevalent name for 'struct
> drm_device' members.
>
> [1] https://patchwork.freedesktop.org/patch/342472/?series=70039=1
> [2] https://patchwork.freedesktop.org/patch/343643/?series=70432=1
I read through the provided links, but can't see why is it called 'drm'.
That sounds like a reference to a DRM driver structure to me.
What about attached_dev or consumer_dev or encoder_dev?
Best regards
Thomas
>
> Cc: Daniel Vetter
> Cc: Laurent Pinchart
> Signed-off-by: Mihail Atanassov
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
> drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +-
> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 +-
> drivers/gpu/drm/bridge/cdns-dsi.c| 2 +-
> drivers/gpu/drm/bridge/dumb-vga-dac.c| 2 +-
> .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +-
> drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> drivers/gpu/drm/bridge/panel.c | 2 +-
> drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> drivers/gpu/drm/bridge/sii902x.c | 6 +++---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 6 +++---
> drivers/gpu/drm/bridge/tc358764.c| 4 ++--
> drivers/gpu/drm/bridge/tc358767.c| 6 +++---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c| 2 +-
> drivers/gpu/drm/bridge/ti-tfp410.c | 6 +++---
> drivers/gpu/drm/drm_bridge.c | 12 ++--
> drivers/gpu/drm/i2c/tda998x_drv.c| 2 +-
> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> drivers/gpu/drm/msm/edp/edp_bridge.c | 2 +-
> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> include/drm/drm_bridge.h | 4 ++--
> 22 files changed, 38 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 9e13e466e72c..db7d01cb0923 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -863,7 +863,7 @@ static int adv7511_bridge_attach(struct drm_bridge
> *bridge)
> adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> DRM_CONNECTOR_POLL_DISCONNECT;
>
> - ret = drm_connector_init(bridge->dev, >connector,
> + ret = drm_connector_init(bridge->drm, >connector,
>_connector_funcs,
>DRM_MODE_CONNECTOR_HDMIA);
> if (ret) {
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> index b4f3a923a52a..0e3508aeaa6c 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -541,7 +541,7 @@ static int anx6345_bridge_attach(struct drm_bridge
> *bridge)
> return err;
> }
>
> - err = drm_connector_init(bridge->dev, >connector,
> + err = drm_connector_init(bridge->drm, >connector,
>_connector_funcs,
>DRM_MODE_CONNECTOR_eDP);
> if (err) {
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> index 41867be03751..d5722bc28933 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> @@ -908,7 +908,7 @@ static int anx78xx_bridge_attach(struct drm_bridge
> *bridge)
> return err;
> }
>
> - err = drm_connector_init(bridge->dev, >connector,
> + err = drm_connector_init(bridge->drm, >connector,
>_connector_funcs,
>DRM_MODE_CONNECTOR_DisplayPort);
> if (err) {
> diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
> b/drivers/gpu/drm/bridge/cdns-dsi.c
> index 3a5bd4e7fd1e..f6d7e97de66e 100644
> --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> @@ -651,7 +651,7 @@ static int cdns_dsi_bridge_attach(struct drm_bridge
> *bridge)
> struct cdns_dsi *dsi = input_to_dsi(input);
> struct cdns_dsi_output *output = >output;
>
> - if (!drm_core_check_feature(bridge->dev, DRIVER_ATOMIC)) {
> + if (!drm_core_check_feature(bridge->drm, DRIVER_ATOMIC)) {
> dev_err(dsi->base.dev,
> "cdns-dsi driver is only compatible with DRM devices
> supporting atomic updates");
> return -ENOTSUPP;
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>
[PATCH] drm: Rename drm_bridge->dev to drm
The 'dev' name causes some confusion with 'struct device' [1][2], so use
'drm' instead since this seems to be the prevalent name for 'struct
drm_device' members.
[1] https://patchwork.freedesktop.org/patch/342472/?series=70039=1
[2] https://patchwork.freedesktop.org/patch/343643/?series=70432=1
Cc: Daniel Vetter
Cc: Laurent Pinchart
Signed-off-by: Mihail Atanassov
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 2 +-
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 +-
drivers/gpu/drm/bridge/cdns-dsi.c| 2 +-
drivers/gpu/drm/bridge/dumb-vga-dac.c| 2 +-
.../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 2 +-
drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
drivers/gpu/drm/bridge/panel.c | 2 +-
drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
drivers/gpu/drm/bridge/sii902x.c | 6 +++---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 6 +++---
drivers/gpu/drm/bridge/tc358764.c| 4 ++--
drivers/gpu/drm/bridge/tc358767.c| 6 +++---
drivers/gpu/drm/bridge/ti-sn65dsi86.c| 2 +-
drivers/gpu/drm/bridge/ti-tfp410.c | 6 +++---
drivers/gpu/drm/drm_bridge.c | 12 ++--
drivers/gpu/drm/i2c/tda998x_drv.c| 2 +-
drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
drivers/gpu/drm/msm/edp/edp_bridge.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
include/drm/drm_bridge.h | 4 ++--
22 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 9e13e466e72c..db7d01cb0923 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -863,7 +863,7 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
DRM_CONNECTOR_POLL_DISCONNECT;
- ret = drm_connector_init(bridge->dev, >connector,
+ ret = drm_connector_init(bridge->drm, >connector,
_connector_funcs,
DRM_MODE_CONNECTOR_HDMIA);
if (ret) {
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
index b4f3a923a52a..0e3508aeaa6c 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
@@ -541,7 +541,7 @@ static int anx6345_bridge_attach(struct drm_bridge *bridge)
return err;
}
- err = drm_connector_init(bridge->dev, >connector,
+ err = drm_connector_init(bridge->drm, >connector,
_connector_funcs,
DRM_MODE_CONNECTOR_eDP);
if (err) {
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
index 41867be03751..d5722bc28933 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
@@ -908,7 +908,7 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge)
return err;
}
- err = drm_connector_init(bridge->dev, >connector,
+ err = drm_connector_init(bridge->drm, >connector,
_connector_funcs,
DRM_MODE_CONNECTOR_DisplayPort);
if (err) {
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
b/drivers/gpu/drm/bridge/cdns-dsi.c
index 3a5bd4e7fd1e..f6d7e97de66e 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -651,7 +651,7 @@ static int cdns_dsi_bridge_attach(struct drm_bridge *bridge)
struct cdns_dsi *dsi = input_to_dsi(input);
struct cdns_dsi_output *output = >output;
- if (!drm_core_check_feature(bridge->dev, DRIVER_ATOMIC)) {
+ if (!drm_core_check_feature(bridge->drm, DRIVER_ATOMIC)) {
dev_err(dsi->base.dev,
"cdns-dsi driver is only compatible with DRM devices
supporting atomic updates");
return -ENOTSUPP;
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index cc33dc411b9e..30b5e54df381 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -112,7 +112,7 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
drm_connector_helper_add(>connector,
_vga_con_helper_funcs);
- ret = drm_connector_init_with_ddc(bridge->dev, >connector,
+ ret =
