From: Hans Verkuil <hverkuil@xs4all.nl> To: Daniel Vetter <daniel@ffwll.ch> Cc: linux-media@vger.kernel.org, linux-samsung-soc@vger.kernel.org, Russell King <linux@armlinux.org.uk>, Krzysztof Kozlowski <krzk@kernel.org>, Javier Martinez Canillas <javier@osg.samsung.com>, dri-devel@lists.freedesktop.org, Daniel Vetter <daniel.vetter@intel.com>, Marek Szyprowski <m.szyprowski@samsung.com> Subject: Re: [PATCHv5 00/11] video/exynos/sti/cec: add CEC notifier & use in drivers Date: Wed, 29 Mar 2017 21:59:34 +0200 [thread overview] Message-ID: <41757527-6953-6095-a18b-e5c75d93c966@xs4all.nl> (raw) In-Reply-To: <20170329174758.55vasy2gxqpg3yb5@phenom.ffwll.local> Hi Daniel, On 29/03/17 19:47, Daniel Vetter wrote: > On Wed, Mar 29, 2017 at 04:15:32PM +0200, Hans Verkuil wrote: >> From: Hans Verkuil <hans.verkuil@cisco.com> >> >> This patch series adds the CEC physical address notifier code, based on >> Russell's code: >> >> https://patchwork.kernel.org/patch/9277043/ >> >> It adds support for it to the exynos_hdmi drm driver, adds support for >> it to the CEC framework and finally adds support to the s5p-cec driver, >> which now can be moved out of staging. >> >> Also included is similar code for the STI platform, contributed by >> Benjamin Gaignard. >> >> Tested the exynos code with my Odroid U3 exynos4 devboard. >> >> After discussions with Daniel Vetter and Russell King I have removed >> the EDID/ELD/HPD connect/disconnect events from the notifier and now >> just use it to report the CEC physical address. This also means that >> it is now renamed to CEC notifier instead of HPD notifier and that >> it is now in drivers/media. The block_notifier was dropped as well >> and instead a simple callback is registered. This means that the >> relationship between HDMI and CEC is now 1:1 and no longer 1:n, but >> should this be needed in the future, then that can easily be added >> back. >> >> Daniel, regarding your suggestions here: >> >> http://www.spinics.net/lists/dri-devel/msg133907.html >> >> this patch series maps to your mail above as follows: >> >> struct cec_pin == struct cec_notifier >> cec_(un)register_pin == cec_notifier_get/put >> cec_set_address == cec_notifier_set_phys_addr >> cec_(un)register_callbacks == cec_notifier_(un)register >> >> Comments are welcome. I'd like to get this in for the 4.12 kernel as >> this is a missing piece needed to integrate CEC drivers. >> >> Regards, >> >> Hans >> >> Changes since v4: >> - Dropped EDID/ELD/connect/disconnect support. Instead, just report the >> CEC physical address (and use INVALID when disconnecting). >> - Since this is now completely CEC specific, move it to drivers/media >> and rename to cec-notifier. >> - Drop block_notifier. Instead just set a callback for the notifier. >> - Use 'hdmi-phandle' in the bindings for both exynos and sti. So no >> vendor prefix and 'hdmi-phandle' instead of 'hdmi-handle'. >> - Make struct cec_notifier opaque. Add a helper function to get the >> physical address from a cec_notifier struct. >> - Provide dummy functions in cec-notifier.h so it can be used when >> CONFIG_MEDIA_CEC_NOTIFIER is undefined. >> - Don't select the CEC notifier in the HDMI drivers. It should only >> be enabled by actual CEC drivers. > > I just quickly scaned through it, but this seems to address all my > concerns fully. Thanks for respinning. On the entire pile (or just the > core cec notifier bits): > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Fantastic! Thank you very much for your comments. One last question: the patches for drivers/gpu/drm: can they go through the media subsystem or do you want to take them? They do depend on the first two patches of this series (cec-edid and cec-notifier), so it is a bit more coordination if they have to go through the drm subsystem. Regards, Hans > >> >> Changes since v3: >> - Added the STI patches >> - Split the exynos4 binding patches in one for documentation and one >> for the dts change itself, also use the correct subject and CC to >> the correct mailinglists (I hope ) >> >> Changes since v2: >> - Split off the dts changes of the s5p-cec patch into a separate patch >> - Renamed HPD_NOTIFIERS to HPD_NOTIFIER to be consistent with the name >> of the source. >> >> Changes since v1: >> >> Renamed HDMI notifier to HPD (hotplug detect) notifier since this code is >> not HDMI specific, but is interesting for any video source that has to >> deal with hotplug detect and EDID/ELD (HDMI, DVI, VGA, DP, ....). >> Only the use with CEC adapters is HDMI specific, but the HPD notifier >> is more generic. >> >> >> >> >> Benjamin Gaignard (4): >> sti: hdmi: add CEC notifier support >> stih-cec.txt: document new hdmi phandle >> stih-cec: add CEC notifier support >> arm: sti: update sti-cec for CEC notifier support >> >> Hans Verkuil (7): >> cec-edid: rename cec_get_edid_phys_addr >> media: add CEC notifier support >> cec: integrate CEC notifier support >> exynos_hdmi: add CEC notifier support >> ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi >> s5p-cec.txt: document the HDMI controller phandle >> s5p-cec: add cec-notifier support, move out of staging >> >> .../devicetree/bindings/media/s5p-cec.txt | 2 + >> .../devicetree/bindings/media/stih-cec.txt | 2 + >> MAINTAINERS | 4 +- >> arch/arm/boot/dts/exynos4.dtsi | 1 + >> arch/arm/boot/dts/stih407-family.dtsi | 12 --- >> arch/arm/boot/dts/stih410.dtsi | 13 +++ >> drivers/gpu/drm/exynos/exynos_hdmi.c | 20 +++- >> drivers/gpu/drm/sti/sti_hdmi.c | 11 ++ >> drivers/gpu/drm/sti/sti_hdmi.h | 3 + >> drivers/media/Kconfig | 3 + >> drivers/media/Makefile | 4 + >> drivers/media/cec-edid.c | 15 ++- >> drivers/media/cec-notifier.c | 116 +++++++++++++++++++++ >> drivers/media/cec/cec-core.c | 21 ++++ >> drivers/media/i2c/adv7511.c | 5 +- >> drivers/media/i2c/adv7604.c | 3 +- >> drivers/media/i2c/adv7842.c | 2 +- >> drivers/media/platform/Kconfig | 28 +++++ >> drivers/media/platform/Makefile | 2 + >> .../media => media/platform}/s5p-cec/Makefile | 0 >> .../platform}/s5p-cec/exynos_hdmi_cec.h | 0 >> .../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0 >> .../media => media/platform}/s5p-cec/regs-cec.h | 0 >> .../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++++++- >> .../media => media/platform}/s5p-cec/s5p_cec.h | 3 + >> .../st-cec => media/platform/sti/cec}/Makefile | 0 >> .../st-cec => media/platform/sti/cec}/stih-cec.c | 31 +++++- >> drivers/media/platform/vivid/vivid-vid-cap.c | 3 +- >> drivers/staging/media/Kconfig | 4 - >> drivers/staging/media/Makefile | 2 - >> drivers/staging/media/s5p-cec/Kconfig | 9 -- >> drivers/staging/media/s5p-cec/TODO | 7 -- >> drivers/staging/media/st-cec/Kconfig | 8 -- >> drivers/staging/media/st-cec/TODO | 7 -- >> include/media/cec-edid.h | 17 ++- >> include/media/cec-notifier.h | 93 +++++++++++++++++ >> include/media/cec.h | 6 ++ >> 37 files changed, 421 insertions(+), 71 deletions(-) >> create mode 100644 drivers/media/cec-notifier.c >> rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%) >> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%) >> rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%) >> rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%) >> delete mode 100644 drivers/staging/media/s5p-cec/Kconfig >> delete mode 100644 drivers/staging/media/s5p-cec/TODO >> delete mode 100644 drivers/staging/media/st-cec/Kconfig >> delete mode 100644 drivers/staging/media/st-cec/TODO >> create mode 100644 include/media/cec-notifier.h >> >> -- >> 2.11.0 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >
WARNING: multiple messages have this Message-ID (diff)
From: Hans Verkuil <hverkuil@xs4all.nl> To: Daniel Vetter <daniel@ffwll.ch> Cc: linux-samsung-soc@vger.kernel.org, Russell King <linux@armlinux.org.uk>, dri-devel@lists.freedesktop.org, Javier Martinez Canillas <javier@osg.samsung.com>, Krzysztof Kozlowski <krzk@kernel.org>, Daniel Vetter <daniel.vetter@intel.com>, Marek Szyprowski <m.szyprowski@samsung.com>, linux-media@vger.kernel.org Subject: Re: [PATCHv5 00/11] video/exynos/sti/cec: add CEC notifier & use in drivers Date: Wed, 29 Mar 2017 21:59:34 +0200 [thread overview] Message-ID: <41757527-6953-6095-a18b-e5c75d93c966@xs4all.nl> (raw) In-Reply-To: <20170329174758.55vasy2gxqpg3yb5@phenom.ffwll.local> Hi Daniel, On 29/03/17 19:47, Daniel Vetter wrote: > On Wed, Mar 29, 2017 at 04:15:32PM +0200, Hans Verkuil wrote: >> From: Hans Verkuil <hans.verkuil@cisco.com> >> >> This patch series adds the CEC physical address notifier code, based on >> Russell's code: >> >> https://patchwork.kernel.org/patch/9277043/ >> >> It adds support for it to the exynos_hdmi drm driver, adds support for >> it to the CEC framework and finally adds support to the s5p-cec driver, >> which now can be moved out of staging. >> >> Also included is similar code for the STI platform, contributed by >> Benjamin Gaignard. >> >> Tested the exynos code with my Odroid U3 exynos4 devboard. >> >> After discussions with Daniel Vetter and Russell King I have removed >> the EDID/ELD/HPD connect/disconnect events from the notifier and now >> just use it to report the CEC physical address. This also means that >> it is now renamed to CEC notifier instead of HPD notifier and that >> it is now in drivers/media. The block_notifier was dropped as well >> and instead a simple callback is registered. This means that the >> relationship between HDMI and CEC is now 1:1 and no longer 1:n, but >> should this be needed in the future, then that can easily be added >> back. >> >> Daniel, regarding your suggestions here: >> >> http://www.spinics.net/lists/dri-devel/msg133907.html >> >> this patch series maps to your mail above as follows: >> >> struct cec_pin == struct cec_notifier >> cec_(un)register_pin == cec_notifier_get/put >> cec_set_address == cec_notifier_set_phys_addr >> cec_(un)register_callbacks == cec_notifier_(un)register >> >> Comments are welcome. I'd like to get this in for the 4.12 kernel as >> this is a missing piece needed to integrate CEC drivers. >> >> Regards, >> >> Hans >> >> Changes since v4: >> - Dropped EDID/ELD/connect/disconnect support. Instead, just report the >> CEC physical address (and use INVALID when disconnecting). >> - Since this is now completely CEC specific, move it to drivers/media >> and rename to cec-notifier. >> - Drop block_notifier. Instead just set a callback for the notifier. >> - Use 'hdmi-phandle' in the bindings for both exynos and sti. So no >> vendor prefix and 'hdmi-phandle' instead of 'hdmi-handle'. >> - Make struct cec_notifier opaque. Add a helper function to get the >> physical address from a cec_notifier struct. >> - Provide dummy functions in cec-notifier.h so it can be used when >> CONFIG_MEDIA_CEC_NOTIFIER is undefined. >> - Don't select the CEC notifier in the HDMI drivers. It should only >> be enabled by actual CEC drivers. > > I just quickly scaned through it, but this seems to address all my > concerns fully. Thanks for respinning. On the entire pile (or just the > core cec notifier bits): > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Fantastic! Thank you very much for your comments. One last question: the patches for drivers/gpu/drm: can they go through the media subsystem or do you want to take them? They do depend on the first two patches of this series (cec-edid and cec-notifier), so it is a bit more coordination if they have to go through the drm subsystem. Regards, Hans > >> >> Changes since v3: >> - Added the STI patches >> - Split the exynos4 binding patches in one for documentation and one >> for the dts change itself, also use the correct subject and CC to >> the correct mailinglists (I hope ) >> >> Changes since v2: >> - Split off the dts changes of the s5p-cec patch into a separate patch >> - Renamed HPD_NOTIFIERS to HPD_NOTIFIER to be consistent with the name >> of the source. >> >> Changes since v1: >> >> Renamed HDMI notifier to HPD (hotplug detect) notifier since this code is >> not HDMI specific, but is interesting for any video source that has to >> deal with hotplug detect and EDID/ELD (HDMI, DVI, VGA, DP, ....). >> Only the use with CEC adapters is HDMI specific, but the HPD notifier >> is more generic. >> >> >> >> >> Benjamin Gaignard (4): >> sti: hdmi: add CEC notifier support >> stih-cec.txt: document new hdmi phandle >> stih-cec: add CEC notifier support >> arm: sti: update sti-cec for CEC notifier support >> >> Hans Verkuil (7): >> cec-edid: rename cec_get_edid_phys_addr >> media: add CEC notifier support >> cec: integrate CEC notifier support >> exynos_hdmi: add CEC notifier support >> ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi >> s5p-cec.txt: document the HDMI controller phandle >> s5p-cec: add cec-notifier support, move out of staging >> >> .../devicetree/bindings/media/s5p-cec.txt | 2 + >> .../devicetree/bindings/media/stih-cec.txt | 2 + >> MAINTAINERS | 4 +- >> arch/arm/boot/dts/exynos4.dtsi | 1 + >> arch/arm/boot/dts/stih407-family.dtsi | 12 --- >> arch/arm/boot/dts/stih410.dtsi | 13 +++ >> drivers/gpu/drm/exynos/exynos_hdmi.c | 20 +++- >> drivers/gpu/drm/sti/sti_hdmi.c | 11 ++ >> drivers/gpu/drm/sti/sti_hdmi.h | 3 + >> drivers/media/Kconfig | 3 + >> drivers/media/Makefile | 4 + >> drivers/media/cec-edid.c | 15 ++- >> drivers/media/cec-notifier.c | 116 +++++++++++++++++++++ >> drivers/media/cec/cec-core.c | 21 ++++ >> drivers/media/i2c/adv7511.c | 5 +- >> drivers/media/i2c/adv7604.c | 3 +- >> drivers/media/i2c/adv7842.c | 2 +- >> drivers/media/platform/Kconfig | 28 +++++ >> drivers/media/platform/Makefile | 2 + >> .../media => media/platform}/s5p-cec/Makefile | 0 >> .../platform}/s5p-cec/exynos_hdmi_cec.h | 0 >> .../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0 >> .../media => media/platform}/s5p-cec/regs-cec.h | 0 >> .../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++++++- >> .../media => media/platform}/s5p-cec/s5p_cec.h | 3 + >> .../st-cec => media/platform/sti/cec}/Makefile | 0 >> .../st-cec => media/platform/sti/cec}/stih-cec.c | 31 +++++- >> drivers/media/platform/vivid/vivid-vid-cap.c | 3 +- >> drivers/staging/media/Kconfig | 4 - >> drivers/staging/media/Makefile | 2 - >> drivers/staging/media/s5p-cec/Kconfig | 9 -- >> drivers/staging/media/s5p-cec/TODO | 7 -- >> drivers/staging/media/st-cec/Kconfig | 8 -- >> drivers/staging/media/st-cec/TODO | 7 -- >> include/media/cec-edid.h | 17 ++- >> include/media/cec-notifier.h | 93 +++++++++++++++++ >> include/media/cec.h | 6 ++ >> 37 files changed, 421 insertions(+), 71 deletions(-) >> create mode 100644 drivers/media/cec-notifier.c >> rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%) >> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%) >> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%) >> rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%) >> rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%) >> delete mode 100644 drivers/staging/media/s5p-cec/Kconfig >> delete mode 100644 drivers/staging/media/s5p-cec/TODO >> delete mode 100644 drivers/staging/media/st-cec/Kconfig >> delete mode 100644 drivers/staging/media/st-cec/TODO >> create mode 100644 include/media/cec-notifier.h >> >> -- >> 2.11.0 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-03-29 19:59 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-29 14:15 [PATCHv5 00/11] video/exynos/sti/cec: add CEC notifier & use in drivers Hans Verkuil 2017-03-29 14:15 ` Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 01/11] cec-edid: rename cec_get_edid_phys_addr Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 02/11] media: add CEC notifier support Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 03/11] cec: integrate " Hans Verkuil 2017-03-29 15:44 ` [PATCHv5.1 " Hans Verkuil 2017-03-29 15:44 ` Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 04/11] exynos_hdmi: add " Hans Verkuil 2017-03-29 14:15 ` Hans Verkuil 2017-03-30 21:37 ` Krzysztof Kozlowski 2017-03-30 22:35 ` Russell King - ARM Linux 2017-03-31 8:05 ` Hans Verkuil 2017-03-31 8:05 ` Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 05/11] ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi Hans Verkuil 2017-03-30 21:34 ` Krzysztof Kozlowski 2017-03-29 14:15 ` [PATCHv5 06/11] s5p-cec.txt: document the HDMI controller phandle Hans Verkuil 2017-03-29 14:15 ` Hans Verkuil 2017-03-30 21:37 ` Krzysztof Kozlowski 2017-03-29 14:15 ` [PATCHv5 07/11] s5p-cec: add cec-notifier support, move out of staging Hans Verkuil 2017-03-30 21:38 ` Krzysztof Kozlowski 2017-03-29 14:15 ` [PATCHv5 08/11] sti: hdmi: add CEC notifier support Hans Verkuil 2017-03-29 14:15 ` Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 09/11] stih-cec.txt: document new hdmi phandle Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 10/11] stih-cec: add CEC notifier support Hans Verkuil 2017-03-29 14:15 ` Hans Verkuil 2017-03-29 14:15 ` [PATCHv5 11/11] arm: sti: update sti-cec for " Hans Verkuil 2017-03-30 7:41 ` Benjamin Gaignard 2017-03-30 7:41 ` Benjamin Gaignard [not found] ` <CA+M3ks442wftNR8+dctdSkKMCPSw9Rd2CH5UG-VEP7XySGjCjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-03-30 9:30 ` Patrice CHOTARD 2017-03-30 9:30 ` Patrice CHOTARD 2017-03-30 9:32 ` Hans Verkuil 2017-03-29 15:55 ` [PATCHv5 00/11] video/exynos/sti/cec: add CEC notifier & use in drivers Benjamin Gaignard 2017-03-29 15:55 ` Benjamin Gaignard 2017-03-29 17:47 ` Daniel Vetter 2017-03-29 17:47 ` Daniel Vetter 2017-03-29 19:59 ` Hans Verkuil [this message] 2017-03-29 19:59 ` Hans Verkuil 2017-03-30 7:47 ` Daniel Vetter 2017-03-30 7:47 ` Daniel Vetter 2017-03-30 20:29 ` Krzysztof Kozlowski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=41757527-6953-6095-a18b-e5c75d93c966@xs4all.nl \ --to=hverkuil@xs4all.nl \ --cc=daniel.vetter@intel.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=javier@osg.samsung.com \ --cc=krzk@kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=m.szyprowski@samsung.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.