mbox series

[v6,00/24] Associate ddc adapters with connectors

Message ID cover.1564161140.git.andrzej.p@collabora.com (mailing list archive)
Headers show
Series Associate ddc adapters with connectors | expand

Message

Andrzej Pietrasiewicz July 26, 2019, 5:22 p.m. UTC
It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
	-> ../../../../soc/13880000.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness): current value =    90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

Patch 2 adds a new variant of drm_connector_init(), see the changelog
below.

Patches 3..24 are examples of how to convert a driver to this new scheme.

v1..v2:

- used fixed name "ddc" for the symbolic link in order to make it easy for
userspace to find the i2c adapter

v2..v3:

- converted as many drivers as possible.

v3..v4:

- added Reviewed-by for patch 01/23
- moved "ddc" field assignment to before drm_connector_init() is called
in msm, vc4, sti, mgag200, ast, amdgpu, radeon
- simplified the code in amdgpu and radeon at the expense of some lines
exceeding 80 characters as per Alex Deucher's suggestion
- added i915

v4..v5:

- changed "include <linux/i2c.h>" to "struct i2c_adapter;"
in drm_connector.h, consequently, added "include <linux/i2c.h>"
in drm_sysfs.c.
- added "drm_connector_init_with_ddc()" variant to ensure that the ddc
field of drm_connector is preserved accross its invocation
- accordingly changed invocations of drm_connector_init() in the
touched drivers to use the new variant

v5..v6:

- improved subject line of patch 1
- added kernel-doc for drm_connector_init_with_ddc()
- improved kernel-doc for the ddc field of struct drm_connector
- added Reviewed-by in patches 17 and 18
- added Acked-by in patch 2
- made the ownership of ddc i2c_adapter explicit in all patches,
this made the affected patches much simpler

@Benjamin
@Shawn

There were your Acked-by or Reviewed-by for some patches in v4, but now
that the patches use the newly added function I'm not sure I can still
include those tags without you actually confirming. Can I? Or can you
please re-review? 

TODO: nouveau, gma500, omapdrm, panel-simple - if applicable.
Other drivers are either already converted or don't mention neither
"ddc" nor "i2c_adapter".

Andrzej Pietrasiewicz (24):
  drm: Add ddc link in sysfs created by drm_connector
  drm: Add drm_connector_init() variant with ddc
  drm/exynos: Provide ddc symlink in connector's sysfs
  drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory
  drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory
  drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
  drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs
    directory
  drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory
  drm/tegra: Provide ddc symlink in output connector sysfs directory
  drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs
  drm/imx: imx-tve: Provide ddc symlink in connector's sysfs
  drm/vc4: Provide ddc symlink in connector sysfs directory
  drm: zte: Provide ddc symlink in hdmi connector sysfs directory
  drm: zte: Provide ddc symlink in vga connector sysfs directory
  drm/tilcdc: Provide ddc symlink in connector sysfs directory
  drm: sti: Provide ddc symlink in hdmi connector sysfs directory
  drm/mgag200: Provide ddc symlink in connector sysfs directory
  drm/ast: Provide ddc symlink in connector sysfs directory
  drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs
    directory
  drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
  drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs
    directory
  drm/amdgpu: Provide ddc symlink in connector sysfs directory
  drm/radeon: Provide ddc symlink in connector sysfs directory
  drm/i915: Provide ddc symlink in hdmi connector sysfs directory

 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    |  96 ++++++++----
 drivers/gpu/drm/ast/ast_mode.c                |  13 +-
 drivers/gpu/drm/bridge/dumb-vga-dac.c         |   6 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     |   6 +-
 drivers/gpu/drm/bridge/ti-tfp410.c            |   6 +-
 drivers/gpu/drm/drm_connector.c               |  35 +++++
 drivers/gpu/drm/drm_sysfs.c                   |   8 +
 drivers/gpu/drm/exynos/exynos_hdmi.c          |   6 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  12 +-
 drivers/gpu/drm/imx/imx-ldb.c                 |   7 +-
 drivers/gpu/drm/imx/imx-tve.c                 |   6 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c           |   7 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c        |  13 +-
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c     |   6 +-
 drivers/gpu/drm/radeon/radeon_connectors.c    | 142 +++++++++++++-----
 drivers/gpu/drm/rockchip/inno_hdmi.c          |   6 +-
 drivers/gpu/drm/rockchip/rk3066_hdmi.c        |   7 +-
 drivers/gpu/drm/sti/sti_hdmi.c                |   6 +-
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c        |   7 +-
 drivers/gpu/drm/tegra/hdmi.c                  |   7 +-
 drivers/gpu/drm/tegra/sor.c                   |   7 +-
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c        |   6 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c                |  12 +-
 drivers/gpu/drm/zte/zx_hdmi.c                 |   6 +-
 drivers/gpu/drm/zte/zx_vga.c                  |   6 +-
 include/drm/drm_connector.h                   |  18 +++
 26 files changed, 336 insertions(+), 121 deletions(-)

Comments

Sam Ravnborg July 26, 2019, 6:35 p.m. UTC | #1
Hi Andezej.

On Fri, Jul 26, 2019 at 07:22:54PM +0200, Andrzej Pietrasiewicz wrote:
> It is difficult for a user to know which of the i2c adapters is for which
> drm connector. This series addresses this problem.
> 
> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> ddcutil:
> 
> ddcutil -b 2 getvcp 0x10
> VCP code 0x10 (Brightness): current value =    90, max value =   100
> 
> The first patch in the series adds struct i2c_adapter pointer to struct
> drm_connector. If the field is used by a particular driver, then an
> appropriate symbolic link is created by the generic code, which is also added
> by this patch.
> 
> Patch 2 adds a new variant of drm_connector_init(), see the changelog
> below.
> 
> Patches 3..24 are examples of how to convert a driver to this new scheme.
> 
...
> 
> v5..v6:
> 
> - improved subject line of patch 1
> - added kernel-doc for drm_connector_init_with_ddc()
> - improved kernel-doc for the ddc field of struct drm_connector
> - added Reviewed-by in patches 17 and 18
> - added Acked-by in patch 2
> - made the ownership of ddc i2c_adapter explicit in all patches,
> this made the affected patches much simpler

Looks good now.
Patch 1 and 2 are:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

The remaining patches are:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam
Neil Armstrong July 31, 2019, 8 a.m. UTC | #2
Hi Sam,

On 26/07/2019 20:55, Sam Ravnborg wrote:
> Hi all.
> 
> Andrzej have done a good job following up on feedback and this series is
> now ready.
> 
> We need ack on the patches touching the individual drivers before we can
> proceed.
> Please check your drivers and get back.

I can apply all core and maintainer-acked patches for now :
1, 2, 7, 10, 11, 16, 17, 18, 19, 20, 21, 22, 23

and Andrzej can resend not applied patches with Yours and Emil's Reviewed-by,
so we can wait a few more days to apply them.

Neil

> 
> 	Sam
> 
>> Hi Andezej.
>>
>> On Fri, Jul 26, 2019 at 07:22:54PM +0200, Andrzej Pietrasiewicz wrote:
>>> It is difficult for a user to know which of the i2c adapters is for which
>>> drm connector. This series addresses this problem.
>>>
>>> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
>>>
>>> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
>>> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
>>> 	-> ../../../../soc/13880000.i2c/i2c-2
>>>
>>> The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
>>> ddcutil:
>>>
>>> ddcutil -b 2 getvcp 0x10
>>> VCP code 0x10 (Brightness): current value =    90, max value =   100
>>>
>>> The first patch in the series adds struct i2c_adapter pointer to struct
>>> drm_connector. If the field is used by a particular driver, then an
>>> appropriate symbolic link is created by the generic code, which is also added
>>> by this patch.
>>>
>>> Patch 2 adds a new variant of drm_connector_init(), see the changelog
>>> below.
>>>
>>> Patches 3..24 are examples of how to convert a driver to this new scheme.
>>>
>> ...
>>>
>>> v5..v6:
>>>
>>> - improved subject line of patch 1
>>> - added kernel-doc for drm_connector_init_with_ddc()
>>> - improved kernel-doc for the ddc field of struct drm_connector
>>> - added Reviewed-by in patches 17 and 18
>>> - added Acked-by in patch 2
>>> - made the ownership of ddc i2c_adapter explicit in all patches,
>>> this made the affected patches much simpler
>>
>> Looks good now.
>> Patch 1 and 2 are:
>> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>>
>> The remaining patches are:
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>>
>> 	Sam
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sam Ravnborg July 31, 2019, 10:40 a.m. UTC | #3
Hi Neil.

On Wed, Jul 31, 2019 at 10:00:14AM +0200, Neil Armstrong wrote:
> Hi Sam,
> 
> On 26/07/2019 20:55, Sam Ravnborg wrote:
> > Hi all.
> > 
> > Andrzej have done a good job following up on feedback and this series is
> > now ready.
> > 
> > We need ack on the patches touching the individual drivers before we can
> > proceed.
> > Please check your drivers and get back.
> 
> I can apply all core and maintainer-acked patches for now :
> 1, 2, 7, 10, 11, 16, 17, 18, 19, 20, 21, 22, 23
> 
> and Andrzej can resend not applied patches with Yours and Emil's Reviewed-by,
> so we can wait a few more days to apply them.

Sounds like a good plan.
Thanks for thaking care of this.

	Sam
Andrzej Pietrasiewicz July 31, 2019, 1:10 p.m. UTC | #4
W dniu 31.07.2019 o 12:40, Sam Ravnborg pisze:
> Hi Neil.
> 
> On Wed, Jul 31, 2019 at 10:00:14AM +0200, Neil Armstrong wrote:
>> Hi Sam,
>>
>> On 26/07/2019 20:55, Sam Ravnborg wrote:
>>> Hi all.
>>>
>>> Andrzej have done a good job following up on feedback and this series is
>>> now ready.
>>>
>>> We need ack on the patches touching the individual drivers before we can
>>> proceed.
>>> Please check your drivers and get back.
>>
>> I can apply all core and maintainer-acked patches for now :
>> 1, 2, 7, 10, 11, 16, 17, 18, 19, 20, 21, 22, 23
>>
>> and Andrzej can resend not applied patches with Yours and Emil's Reviewed-by,
>> so we can wait a few more days to apply them.
> 
> Sounds like a good plan.
> Thanks for thaking care of this.

When is it good time to resend patches 3, 4, 5, 6, 8, 9, 12, 13, 14, 15, 24 as a
new series?

Andrzej
Neil Armstrong July 31, 2019, 2:22 p.m. UTC | #5
On 31/07/2019 15:10, Andrzej Pietrasiewicz wrote:
> W dniu 31.07.2019 o 12:40, Sam Ravnborg pisze:
>> Hi Neil.
>>
>> On Wed, Jul 31, 2019 at 10:00:14AM +0200, Neil Armstrong wrote:
>>> Hi Sam,
>>>
>>> On 26/07/2019 20:55, Sam Ravnborg wrote:
>>>> Hi all.
>>>>
>>>> Andrzej have done a good job following up on feedback and this series is
>>>> now ready.
>>>>
>>>> We need ack on the patches touching the individual drivers before we can
>>>> proceed.
>>>> Please check your drivers and get back.
>>>
>>> I can apply all core and maintainer-acked patches for now :
>>> 1, 2, 7, 10, 11, 16, 17, 18, 19, 20, 21, 22, 23
>>>
>>> and Andrzej can resend not applied patches with Yours and Emil's Reviewed-by,
>>> so we can wait a few more days to apply them.
>>
>> Sounds like a good plan.
>> Thanks for thaking care of this.
> 
> When is it good time to resend patches 3, 4, 5, 6, 8, 9, 12, 13, 14, 15, 24 as a
> new series?

I'll ping you when everything is applied, build-tested and pushed on drm-misc-next

Neil

> 
> Andrzej
Neil Armstrong July 31, 2019, 3:32 p.m. UTC | #6
Hi Andrzej,

On 31/07/2019 16:22, Neil Armstrong wrote:
> On 31/07/2019 15:10, Andrzej Pietrasiewicz wrote:
>> W dniu 31.07.2019 o 12:40, Sam Ravnborg pisze:
>>> Hi Neil.
>>>
>>> On Wed, Jul 31, 2019 at 10:00:14AM +0200, Neil Armstrong wrote:
>>>> Hi Sam,
>>>>
>>>> On 26/07/2019 20:55, Sam Ravnborg wrote:
>>>>> Hi all.
>>>>>
>>>>> Andrzej have done a good job following up on feedback and this series is
>>>>> now ready.
>>>>>
>>>>> We need ack on the patches touching the individual drivers before we can
>>>>> proceed.
>>>>> Please check your drivers and get back.
>>>>
>>>> I can apply all core and maintainer-acked patches for now :
>>>> 1, 2, 7, 10, 11, 16, 17, 18, 19, 20, 21, 22, 23
>>>>
>>>> and Andrzej can resend not applied patches with Yours and Emil's Reviewed-by,
>>>> so we can wait a few more days to apply them.
>>>
>>> Sounds like a good plan.
>>> Thanks for thaking care of this.
>>
>> When is it good time to resend patches 3, 4, 5, 6, 8, 9, 12, 13, 14, 15, 24 as a
>> new series?
> 
> I'll ping you when everything is applied, build-tested and pushed on drm-misc-next

I pushed 1, 2, 7, 10, 11, 16, 17, 18, 19, 20, 21, 22, 23 :
bed7a2182de6 drm/radeon: Provide ddc symlink in connector sysfs directory
5b50fa2b35a4 drm/amdgpu: Provide ddc symlink in connector sysfs directory
cfb444552926 drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs directory
9ebc4d2140ad drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
a4f9087e85de drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs directory
350fd554ee44 drm/ast: Provide ddc symlink in connector sysfs directory
9572ae176a10 drm/mgag200: Provide ddc symlink in connector sysfs directory
7058e76682d7 drm: sti: Provide ddc symlink in hdmi connector sysfs directory
2ae7eb372ed4 drm/imx: imx-tve: Provide ddc symlink in connector's sysfs
be0ec35940bc drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs
1e8f17855ff8 drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs directory
100163df4203 drm: Add drm_connector_init() variant with ddc
e1a29c6c5955 drm: Add ddc link in sysfs created by drm_connector

Neil

> 
> Neil
> 
>>
>> Andrzej
>
Andrzej Pietrasiewicz July 31, 2019, 4:58 p.m. UTC | #7
Now that some of the patches of the previous v6 series are applied,
I'm resending the remaining ones (patches 3-13) with Acked-by and
Reviewed-by added.

I'm also taking this opportunity to provide the symlink for another
connector in amdgpu (patch 1), and to fix a small but nasty bug
which can cause a use of an uninitialized variable (patch 2).

Andrzej Pietrasiewicz (13):
  drm/amdgpu: Provide ddc symlink in dm connector's sysfs directory
  drm/radeon: Eliminate possible use of an uninitialized variable
  drm/exynos: Provide ddc symlink in connector's sysfs
  drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory
  drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory
  drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
  drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory
  drm/tegra: Provide ddc symlink in output connector sysfs directory
  drm/vc4: Provide ddc symlink in connector sysfs directory
  drm: zte: Provide ddc symlink in hdmi connector sysfs directory
  drm: zte: Provide ddc symlink in vga connector sysfs directory
  drm/tilcdc: Provide ddc symlink in connector sysfs directory
  drm/i915: Provide ddc symlink in hdmi connector sysfs directory

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 +++--
 drivers/gpu/drm/exynos/exynos_hdmi.c              |  6 ++++--
 drivers/gpu/drm/i915/display/intel_hdmi.c         | 12 ++++++++----
 drivers/gpu/drm/mediatek/mtk_hdmi.c               |  7 ++++---
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c         |  6 ++++--
 drivers/gpu/drm/radeon/radeon_connectors.c        |  2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c              |  6 ++++--
 drivers/gpu/drm/rockchip/rk3066_hdmi.c            |  7 ++++---
 drivers/gpu/drm/tegra/hdmi.c                      |  7 ++++---
 drivers/gpu/drm/tegra/sor.c                       |  7 ++++---
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c            |  6 ++++--
 drivers/gpu/drm/vc4/vc4_hdmi.c                    | 12 ++++++++----
 drivers/gpu/drm/zte/zx_hdmi.c                     |  6 ++++--
 drivers/gpu/drm/zte/zx_vga.c                      |  6 ++++--
 14 files changed, 60 insertions(+), 35 deletions(-)
Laurent Pinchart Aug. 4, 2019, 12:33 p.m. UTC | #8
Hi Andrzej,

On Fri, Jul 26, 2019 at 07:22:54PM +0200, Andrzej Pietrasiewicz wrote:
> It is difficult for a user to know which of the i2c adapters is for which
> drm connector. This series addresses this problem.
> 
> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> ddcutil:
> 
> ddcutil -b 2 getvcp 0x10
> VCP code 0x10 (Brightness): current value =    90, max value =   100
> 
> The first patch in the series adds struct i2c_adapter pointer to struct
> drm_connector. If the field is used by a particular driver, then an
> appropriate symbolic link is created by the generic code, which is also added
> by this patch.
> 
> Patch 2 adds a new variant of drm_connector_init(), see the changelog
> below.
> 
> Patches 3..24 are examples of how to convert a driver to this new scheme.
> 
> v1..v2:
> 
> - used fixed name "ddc" for the symbolic link in order to make it easy for
> userspace to find the i2c adapter
> 
> v2..v3:
> 
> - converted as many drivers as possible.
> 
> v3..v4:
> 
> - added Reviewed-by for patch 01/23
> - moved "ddc" field assignment to before drm_connector_init() is called
> in msm, vc4, sti, mgag200, ast, amdgpu, radeon
> - simplified the code in amdgpu and radeon at the expense of some lines
> exceeding 80 characters as per Alex Deucher's suggestion
> - added i915
> 
> v4..v5:
> 
> - changed "include <linux/i2c.h>" to "struct i2c_adapter;"
> in drm_connector.h, consequently, added "include <linux/i2c.h>"
> in drm_sysfs.c.
> - added "drm_connector_init_with_ddc()" variant to ensure that the ddc
> field of drm_connector is preserved accross its invocation
> - accordingly changed invocations of drm_connector_init() in the
> touched drivers to use the new variant
> 
> v5..v6:
> 
> - improved subject line of patch 1
> - added kernel-doc for drm_connector_init_with_ddc()
> - improved kernel-doc for the ddc field of struct drm_connector
> - added Reviewed-by in patches 17 and 18
> - added Acked-by in patch 2
> - made the ownership of ddc i2c_adapter explicit in all patches,
> this made the affected patches much simpler
> 
> @Benjamin
> @Shawn
> 
> There were your Acked-by or Reviewed-by for some patches in v4, but now
> that the patches use the newly added function I'm not sure I can still
> include those tags without you actually confirming. Can I? Or can you
> please re-review? 
> 
> TODO: nouveau, gma500, omapdrm, panel-simple - if applicable.

omapdrm is moving to a new helper that creates connectors for a set of
bridges, so I'll handle it there. It may require adding a ddc field to
drm_bridge.

> Other drivers are either already converted or don't mention neither
> "ddc" nor "i2c_adapter".
> 
> Andrzej Pietrasiewicz (24):
>   drm: Add ddc link in sysfs created by drm_connector
>   drm: Add drm_connector_init() variant with ddc
>   drm/exynos: Provide ddc symlink in connector's sysfs
>   drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory
>   drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory
>   drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
>   drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs
>     directory
>   drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory
>   drm/tegra: Provide ddc symlink in output connector sysfs directory
>   drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs
>   drm/imx: imx-tve: Provide ddc symlink in connector's sysfs
>   drm/vc4: Provide ddc symlink in connector sysfs directory
>   drm: zte: Provide ddc symlink in hdmi connector sysfs directory
>   drm: zte: Provide ddc symlink in vga connector sysfs directory
>   drm/tilcdc: Provide ddc symlink in connector sysfs directory
>   drm: sti: Provide ddc symlink in hdmi connector sysfs directory
>   drm/mgag200: Provide ddc symlink in connector sysfs directory
>   drm/ast: Provide ddc symlink in connector sysfs directory
>   drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs
>     directory
>   drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
>   drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs
>     directory
>   drm/amdgpu: Provide ddc symlink in connector sysfs directory
>   drm/radeon: Provide ddc symlink in connector sysfs directory
>   drm/i915: Provide ddc symlink in hdmi connector sysfs directory
> 
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    |  96 ++++++++----
>  drivers/gpu/drm/ast/ast_mode.c                |  13 +-
>  drivers/gpu/drm/bridge/dumb-vga-dac.c         |   6 +-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     |   6 +-
>  drivers/gpu/drm/bridge/ti-tfp410.c            |   6 +-
>  drivers/gpu/drm/drm_connector.c               |  35 +++++
>  drivers/gpu/drm/drm_sysfs.c                   |   8 +
>  drivers/gpu/drm/exynos/exynos_hdmi.c          |   6 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c     |  12 +-
>  drivers/gpu/drm/imx/imx-ldb.c                 |   7 +-
>  drivers/gpu/drm/imx/imx-tve.c                 |   6 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   7 +-
>  drivers/gpu/drm/mgag200/mgag200_mode.c        |  13 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_connector.c     |   6 +-
>  drivers/gpu/drm/radeon/radeon_connectors.c    | 142 +++++++++++++-----
>  drivers/gpu/drm/rockchip/inno_hdmi.c          |   6 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c        |   7 +-
>  drivers/gpu/drm/sti/sti_hdmi.c                |   6 +-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c        |   7 +-
>  drivers/gpu/drm/tegra/hdmi.c                  |   7 +-
>  drivers/gpu/drm/tegra/sor.c                   |   7 +-
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c        |   6 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c                |  12 +-
>  drivers/gpu/drm/zte/zx_hdmi.c                 |   6 +-
>  drivers/gpu/drm/zte/zx_vga.c                  |   6 +-
>  include/drm/drm_connector.h                   |  18 +++
>  26 files changed, 336 insertions(+), 121 deletions(-)
Guenter Roeck Aug. 8, 2019, 3:42 a.m. UTC | #9
On Fri, Jul 26, 2019 at 07:23:13PM +0200, Andrzej Pietrasiewicz wrote:
> Use the ddc pointer provided by the generic connector.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

This patch results in a crash when running qemu:versatilepb.

Unable to handle kernel NULL pointer dereference at virtual address 000000c5
pgd = (ptrval)
[000000c5] *pgd=00000000
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 5.3.0-rc1+ #1
Hardware name: ARM-Versatile (Device Tree Support)
PC is at sysfs_do_create_link_sd+0x38/0xd8
LR is at sysfs_do_create_link_sd+0x38/0xd8
pc : [<c01ac94c>]    lr : [<c01ac94c>]    psr: a0000153
sp : c783bd18  ip : 00000000  fp : c783bde8
r10: c7ef5ea8  r9 : 00000001  r8 : c0955dc0
r7 : c73cb5b0  r6 : c73cd800  r5 : 000000ad  r4 : 00000000
r3 : c7838ae0  r2 : 00000000  r1 : 00000008  r0 : c0aa2898
Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
Control: 00093177  Table: 00004000  DAC: 00000053
Process swapper (pid: 1, stack limit = 0x(ptrval))
Stack: (0xc783bd18 to 0xc783c000)
bd00:                                                       c73ccc48 c73ccc74
bd20: c73cd800 c0ac7c88 00000000 c729cc80 c7ef5ea8 c04c7fc0 c73ccc48 c0a73068
bd40: c73cd800 c0ac7c88 00000000 c04c87e0 00000001 00000000 c04cefcc c04dc3f8
bd60: c73a9030 c73cd800 c73ccc48 7fc2ce37 00000000 c73cd800 00000000 c04cefcc
bd80: c73cd800 00000000 00000000 c04b4ebc c0a73068 c7ef5ea8 c783bde8 c049ffcc
bda0: c73a9020 c73cd800 c78e6000 c73a9020 00000000 c73a9020 c0a73068 c04df2f8
bdc0: c783bde8 c095a76c c73a9020 c0065744 c73ccc20 c73a9020 00000000 00000001
bde0: c7838ae0 00000000 c73ccc20 7fc2ce37 00000000 c78e6000 00000000 c0ac7c34
be00: c07dc1f8 00000000 00000000 c0a6b384 c0a59858 c045e8d8 c78e6000 c1173a78
be20: 00000000 c0ac7c34 00000000 c04e77c4 c78e6000 c0ac7c34 c0ac7c34 c0a73068
be40: 00000000 ffffe000 c0a6b384 c04e7a34 c0ac7c34 c0ac7c34 c0a73068 c78e6000
be60: 00000000 c0ac7c34 c0a73068 00000000 ffffe000 c0a6b384 c0a59858 c04e7cf0
be80: 00000000 c0ac7c34 c78e6000 c04e7d7c 00000000 c0ac7c34 c04e7cf8 c04e5928
bea0: c73b2800 c78d88a0 c78dd110 7fc2ce37 ffffe000 c0ac7c34 c73b2800 c0ac16e0
bec0: 00000000 c04e6b28 c095a73c c0af0a60 c0a73068 c0ac7c34 c0af0a60 c0a73068
bee0: c0a401c4 c04e8968 ffffe000 c0af0a60 c0a73068 c000b3bc 00000115 00000000
bf00: c7ffce6c c7ffce00 c09e15b0 00000115 00000115 c0048844 c09e000c c097cfd4
bf20: 00000000 00000006 00000006 00000000 00000000 c7ffce6c ffffe000 c006954c
bf40: ffffe000 7fc2ce37 c0afb000 c0af0a60 00000115 c0afb000 00000007 c0a59850
bf60: ffffe000 c0a111e8 00000006 00000006 00000000 c0a10678 00000000 7fc2ce37
bf80: 00000000 00000000 c07824cc 00000000 00000000 00000000 00000000 00000000
bfa0: 00000000 c07824d4 00000000 c00090b0 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[<c01ac94c>] (sysfs_do_create_link_sd) from [<c04c7fc0>] (drm_connector_register.part.1+0x40/0xa0)
[<c04c7fc0>] (drm_connector_register.part.1) from [<c04c87e0>] (drm_connector_register_all+0x90/0xb8)
[<c04c87e0>] (drm_connector_register_all) from [<c04cefcc>] (drm_modeset_register_all+0x44/0x6c)
[<c04cefcc>] (drm_modeset_register_all) from [<c04b4ebc>] (drm_dev_register+0x15c/0x1c0)
[<c04b4ebc>] (drm_dev_register) from [<c04df2f8>] (pl111_amba_probe+0x2e0/0x4ac)
[<c04df2f8>] (pl111_amba_probe) from [<c045e8d8>] (amba_probe+0x9c/0x118)
[<c045e8d8>] (amba_probe) from [<c04e77c4>] (really_probe+0x1c0/0x2bc)
[<c04e77c4>] (really_probe) from [<c04e7a34>] (driver_probe_device+0x5c/0x170)
[<c04e7a34>] (driver_probe_device) from [<c04e7cf0>] (device_driver_attach+0x58/0x60)
[<c04e7cf0>] (device_driver_attach) from [<c04e7d7c>] (__driver_attach+0x84/0xc0)
[<c04e7d7c>] (__driver_attach) from [<c04e5928>] (bus_for_each_dev+0x70/0xb4)
[<c04e5928>] (bus_for_each_dev) from [<c04e6b28>] (bus_add_driver+0x154/0x1e0)
[<c04e6b28>] (bus_add_driver) from [<c04e8968>] (driver_register+0x74/0x108)
[<c04e8968>] (driver_register) from [<c000b3bc>] (do_one_initcall+0x84/0x2e4)
[<c000b3bc>] (do_one_initcall) from [<c0a111e8>] (kernel_init_freeable+0x2bc/0x394)
[<c0a111e8>] (kernel_init_freeable) from [<c07824d4>] (kernel_init+0x8/0xf0)
[<c07824d4>] (kernel_init) from [<c00090b0>] (ret_from_fork+0x14/0x24)
Exception stack(0xc783bfb0 to 0xc783bff8)
bfa0:                                     00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
Code: e59f00a0 e1a09003 e1a08002 eb176e54 (e5955018) 
---[ end trace f503b374936886c5 ]---

Bisect log attached.

Guenter

---
# bad: [3880be629e26f6c407593602398c6651860d5fae] Add linux-next specific files for 20190807
# good: [e21a712a9685488f5ce80495b37b9fdbe96c230d] Linux 5.3-rc3
git bisect start 'HEAD' 'v5.3-rc3'
# good: [83d74da9e6d2ca78b32e9e794c6bcbd433d5efaa] Merge remote-tracking branch 'crypto/master'
git bisect good 83d74da9e6d2ca78b32e9e794c6bcbd433d5efaa
# bad: [3add021bff629f1792a5e4268afe13b3047b5523] Merge remote-tracking branch 'sound/for-next'
git bisect bad 3add021bff629f1792a5e4268afe13b3047b5523
# good: [4ef58ee18a654b1992d00281501d6eff051a0c5e] Merge remote-tracking branch 'amdgpu/drm-next'
git bisect good 4ef58ee18a654b1992d00281501d6eff051a0c5e
# good: [f729d8d9628d4093675abfd62d8d3a06d3d11732] drm/tinydrm: Move tinydrm_machine_little_endian()
git bisect good f729d8d9628d4093675abfd62d8d3a06d3d11732
# bad: [178e5f3a5bc1d67d1248a74c0abab41040abe7c4] drm/crc-debugfs: Add notes about CRC<->commit interactions
git bisect bad 178e5f3a5bc1d67d1248a74c0abab41040abe7c4
# good: [0486ad20e73d03c82208b802ac41e80b942b23f6] drm/rockchip: Make analogix_dp_atomic_check static
git bisect good 0486ad20e73d03c82208b802ac41e80b942b23f6
# bad: [92cb3e5980638a37c56091e605aa837d0af05a9d] dma-buf: fix stack corruption in dma_fence_chain_release
git bisect bad 92cb3e5980638a37c56091e605aa837d0af05a9d
# good: [350fd554ee44325661d0d3c3831f428f4fbb0f2d] drm/ast: Provide ddc symlink in connector sysfs directory
git bisect good 350fd554ee44325661d0d3c3831f428f4fbb0f2d
# bad: [28ba1b1da49a20ba8fb767d6ddd7c521ec79a119] drm: mali-dp: Mark expected switch fall-through
git bisect bad 28ba1b1da49a20ba8fb767d6ddd7c521ec79a119
# bad: [5b50fa2b35a4ddad11cb3d06231bf71759b49566] drm/amdgpu: Provide ddc symlink in connector sysfs directory
git bisect bad 5b50fa2b35a4ddad11cb3d06231bf71759b49566
# bad: [9ebc4d2140adcdea2399b42d3f0d2f7e141ac1a8] drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
git bisect bad 9ebc4d2140adcdea2399b42d3f0d2f7e141ac1a8
# bad: [a4f9087e85de141e4e6d21ac2c583ae096cc9aba] drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs directory
git bisect bad a4f9087e85de141e4e6d21ac2c583ae096cc9aba
# first bad commit: [a4f9087e85de141e4e6d21ac2c583ae096cc9aba] drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs directory
Geert Uytterhoeven Aug. 13, 2019, 9:33 a.m. UTC | #10
Hi Günter,

On Thu, Aug 8, 2019 at 5:42 AM Guenter Roeck <linux@roeck-us.net> wrote:
> On Fri, Jul 26, 2019 at 07:23:13PM +0200, Andrzej Pietrasiewicz wrote:
> > Use the ddc pointer provided by the generic connector.
> >
> > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> > Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>
> This patch results in a crash when running qemu:versatilepb.
>
> Unable to handle kernel NULL pointer dereference at virtual address 000000c5
> pgd = (ptrval)
> [000000c5] *pgd=00000000
> Internal error: Oops: 5 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 5.3.0-rc1+ #1
> Hardware name: ARM-Versatile (Device Tree Support)
> PC is at sysfs_do_create_link_sd+0x38/0xd8
> LR is at sysfs_do_create_link_sd+0x38/0xd8

> [<c01ac94c>] (sysfs_do_create_link_sd) from [<c04c7fc0>] (drm_connector_register.part.1+0x40/0xa0)
> [<c04c7fc0>] (drm_connector_register.part.1) from [<c04c87e0>] (drm_connector_register_all+0x90/0xb8)
> [<c04c87e0>] (drm_connector_register_all) from [<c04cefcc>] (drm_modeset_register_all+0x44/0x6c)
> [<c04cefcc>] (drm_modeset_register_all) from [<c04b4ebc>] (drm_dev_register+0x15c/0x1c0)
> [<c04b4ebc>] (drm_dev_register) from [<c04df2f8>] (pl111_amba_probe+0x2e0/0x4ac)
> [<c04df2f8>] (pl111_amba_probe) from [<c045e8d8>] (amba_probe+0x9c/0x118)

Seeing the same thing on Salvator-XS, due to vga->ddc being -ENODEV.

> # first bad commit: [a4f9087e85de141e4e6d21ac2c583ae096cc9aba] drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs directory

Fix sent
https://lore.kernel.org/lkml/20190813093046.4976-1-geert+renesas@glider.be/

Gr{oetje,eeting}s,

                        Geert