| Message ID | cover.1564161140.git.andrzej.p@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Associate ddc adapters with connectors | expand |
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
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. 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
On 2019/07/26, 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. > Other drivers are either already converted or don't mention neither > "ddc" nor "i2c_adapter". > Another way to check is to look for drm_get_edid. Sadly that also highlights aux. dp/mst instances, which expose the DDC in another way. For example comparing the diff stat wrt the following command shows git grep -wc drm_get_edid -- drivers/gpu/drm/ > > .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 96 ++++++++---- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c:1 - as pointed out by Alex - mix of aux dp/mst and normal > drivers/gpu/drm/ast/ast_mode.c | 13 +- drivers/gpu/drm/bridge/analogix-anx78xx.c:1 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1 - not applicable: aux dp/mst > drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +- drivers/gpu/drm/bridge/sii902x.c:1 - normal instance(?) that should be updated at some point. > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 +- drivers/gpu/drm/bridge/tc358767.c:1 - not applicable: aux dp/mst > drivers/gpu/drm/bridge/ti-tfp410.c | 6 +- drivers/gpu/drm/drm_dp_mst_topology.c:3 - not applicable: aux dp/mst > drivers/gpu/drm/drm_connector.c | 35 +++++ > drivers/gpu/drm/drm_sysfs.c | 8 + drivers/gpu/drm/drm_edid.c:7 drivers/gpu/drm/drm_probe_helper.c:1 - unrelated > drivers/gpu/drm/exynos/exynos_hdmi.c | 6 +- drivers/gpu/drm/gma500/cdv_intel_dp.c:3 drivers/gpu/drm/gma500/cdv_intel_hdmi.c:2 drivers/gpu/drm/gma500/oaktrail_hdmi.c:1 drivers/gpu/drm/gma500/oaktrail_lvds.c:2 drivers/gpu/drm/gma500/psb_intel_modes.c:1 drivers/gpu/drm/gma500/psb_intel_sdvo.c:2 - should be updated at some point (as you pointed out). > drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +- drivers/gpu/drm/i915/intel_connector.c:1 drivers/gpu/drm/i915/intel_crt.c:2 - not too sure here drivers/gpu/drm/i915/intel_dp.c:2 - not applicable: aux dp/mst drivers/gpu/drm/i915/intel_lvds.c:1 drivers/gpu/drm/i915/intel_sdvo.c:2 - not too sure here > 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/msm/edp/edp_ctrl.c:1 - not applicable: aux dp/mst drivers/gpu/drm/nouveau/nouveau_connector.c:2 - should be updated at some point (as you pointed out). drivers/gpu/drm/panel/panel-simple.c:1 - no applicable: panel driver > 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/tegra/output.c:1 - already handled in hdmi/sor > 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(-) In a Tl;Dr: I think this series covers 90%+ of the existing rather huge) driverset. For the series: Reviewed-by: Emil Velikov <emil.velikov@collabora.com> Fwiw I'm in favour of Jani's suggestion to fold the dcc into the usual helper drm_connector_init(). Although since we have 130+ instances it might be better left for another day. HTH -Emil
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
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
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
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
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 >
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(-)
On Tue, Jul 30, 2019 at 04:01:23PM +0100, Emil Velikov wrote: > On 2019/07/26, 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. > > Other drivers are either already converted or don't mention neither > > "ddc" nor "i2c_adapter". > > > Another way to check is to look for drm_get_edid. Sadly that also > highlights aux. dp/mst instances, which expose the DDC in another way. > > For example comparing the diff stat wrt the following command shows > git grep -wc drm_get_edid -- drivers/gpu/drm/ There's also drm_do_get_edid, which points to the adv7511 bridge as a good candidate. > > > > .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 96 ++++++++---- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c:1 > - as pointed out by Alex - mix of aux dp/mst and normal > > > drivers/gpu/drm/ast/ast_mode.c | 13 +- > > drivers/gpu/drm/bridge/analogix-anx78xx.c:1 > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1 > - not applicable: aux dp/mst > > > > drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +- > > drivers/gpu/drm/bridge/sii902x.c:1 > - normal instance(?) that should be updated at some point. > > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 +- > > drivers/gpu/drm/bridge/tc358767.c:1 > - not applicable: aux dp/mst > > > drivers/gpu/drm/bridge/ti-tfp410.c | 6 +- > > drivers/gpu/drm/drm_dp_mst_topology.c:3 > - not applicable: aux dp/mst > > > drivers/gpu/drm/drm_connector.c | 35 +++++ > > drivers/gpu/drm/drm_sysfs.c | 8 + > > drivers/gpu/drm/drm_edid.c:7 > drivers/gpu/drm/drm_probe_helper.c:1 > - unrelated > > > drivers/gpu/drm/exynos/exynos_hdmi.c | 6 +- > > drivers/gpu/drm/gma500/cdv_intel_dp.c:3 > drivers/gpu/drm/gma500/cdv_intel_hdmi.c:2 > drivers/gpu/drm/gma500/oaktrail_hdmi.c:1 > drivers/gpu/drm/gma500/oaktrail_lvds.c:2 > drivers/gpu/drm/gma500/psb_intel_modes.c:1 > drivers/gpu/drm/gma500/psb_intel_sdvo.c:2 > - should be updated at some point (as you pointed out). > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +- > drivers/gpu/drm/i915/intel_connector.c:1 > drivers/gpu/drm/i915/intel_crt.c:2 > - not too sure here > > drivers/gpu/drm/i915/intel_dp.c:2 > - not applicable: aux dp/mst > > drivers/gpu/drm/i915/intel_lvds.c:1 > drivers/gpu/drm/i915/intel_sdvo.c:2 > - not too sure here > > > > 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/msm/edp/edp_ctrl.c:1 > - not applicable: aux dp/mst > > drivers/gpu/drm/nouveau/nouveau_connector.c:2 > - should be updated at some point (as you pointed out). > > > drivers/gpu/drm/panel/panel-simple.c:1 > - no applicable: panel driver > > > > 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/tegra/output.c:1 > - already handled in hdmi/sor > > > 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(-) > > In a Tl;Dr: I think this series covers 90%+ of the existing rather huge) driverset. > > For the series: > Reviewed-by: Emil Velikov <emil.velikov@collabora.com> > > Fwiw I'm in favour of Jani's suggestion to fold the dcc into the usual > helper drm_connector_init(). Although since we have 130+ instances it > might be better left for another day. > > HTH > -Emil
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(-)