On Mon, Jul 24, 2017 at 02:16:40PM +0200, Hans Verkuil wrote: > Hi Russell, > > On 07/17/2017 02:23 PM, Hans Verkuil wrote: > >On 17/07/17 14:05, Russell King - ARM Linux wrote: > >>On Mon, Jul 17, 2017 at 01:39:48PM +0200, Hans Verkuil wrote: > >>>On 17/07/17 11:05, Russell King - ARM Linux wrote: > >>>>On Mon, Jul 17, 2017 at 10:56:47AM +0200, Hans Verkuil wrote: > >>>>>Hi Russell, > >>>>> > >>>>>On 09/06/17 16:10, Russell King - ARM Linux wrote: > >>>>>>On Fri, Jun 09, 2017 at 03:56:39PM +0200, Neil Armstrong wrote: > >>>>>>>Yes, but on the Amlogic Meson plarform, the DW-HDMI CEC controller is > >>>>>>>not used, but a custom one, so this notifier is actually useful for > >>>>>>>this platform and maybe others. > >>>>>> > >>>>>>Is the CEC controller configured into dw-hdmi (is the config bit set?) > >>>>>>I'm just wondering if we're going to end up with two CEC drivers trying > >>>>>>to bind to the same notifier. > >>>>>> > >>>>>>>Should we really wait until I push the Amlogic AO CEC driver ? Having a > >>>>>>>notifier in the DW-HDMI driver won't harm anybody since it *will be used*. > >>>>>> > >>>>>>It sounds like this adds additional information that has been missing > >>>>>>from the review of my patches - and I suspect changes Hans' comments. > >>>>>>So, I'll wait, it seems pointless to try and update the patches when > >>>>>>it's not clear how to proceed due to other dependencies, especially > >>>>>>when it means that their existing state is what's required (I'm pleased > >>>>>>that I've held off modifying the patches so far.) > >>>>>> > >>>>>>If that means having to wait another kernel revision, then I guess that's > >>>>>>what will have to happen. > >>>>> > >>>>>Can you respin your patch series, keeping the notifier support? The CEC > >>>>>kernel config handling has been cleaned up (just select CEC_CORE and > >>>>>CEC_NOTIFIER) so you should be good to go. > >>>> > >>>>Not yet - the change to the way you're dealing with Kconfig in CEC is > >>>>fundamentally broken, and needs fixing before we can merge dw-hdmi-cec > >>>>support. > >>>> > >>>>As a result of these Kconfig changes, dw-hdmi-cec now fails if: > >>>> > >>>>1. You build the CEC part as a module > >>>>2. You build the HDMI part into the kernel > >>>> > >>>>This results in CEC_NOTIFIER=y and CEC_CORE=m, which, when the HDMI part > >>>>gets built, results in the stubs in the notifier code being used, rather > >>>>than the real functions. This in turn causes the CEC part to never > >>>>receive a physical address, which is therefore non-functional. > >>>> > >>>>I did have a patch to fix this, but it was never committed, and I got > >>>>busy with other stuff (so it ended up being git reset --hard away.) > >>>> > >>> > >>>This is more a DRM_DW_HDMI issue than a CEC issue IMHO. > >>> > >>>This will fix this: > >>> > >>>config DRM_DW_HDMI > >>> tristate > >>> select DRM_KMS_HELPER > >>> select REGMAP_MMIO > >>> select CEC_CORE if CEC_NOTIFIER <<<<<< > >>> > >>>config DRM_DW_HDMI_CEC > >>> tristate "Synopsis Designware CEC interface" > >>> depends on DRM_DW_HDMI > >>> select CEC_CORE > >>> select CEC_NOTIFIER > >>> help > >>> Support the CE interface which is part of the Synopsis > >>> Designware HDMI block. > >>> > >>>This makes sense: if DRM_DW_HDMI_CEC is disabled but another CEC module is > >>>used instead (as is apparently the case for amlogic), then the > >>> > >>> select CEC_CORE if CEC_NOTIFIER > >>> > >>>line ensures that CONFIG_CEC_CORE has the right m/y value. > >> > >>I disagree with this approach. > >> > >>If DRM_DW_HDMI=y and DRM_DW_HDMI_CEC=n, but some other driver is enabled > >>that selects CEC_NOTIFIER, then we end up with CEC_CORE forced enabled > >>through dw-hdmi, even though we haven't asked for the CEC part to be > >>enabled. > > > >If CEC_NOTIFIER is enabled by a CEC driver, then CEC_CORE will also be > >enabled (without CEC_CORE that driver wouldn't compile, obviously). > > > >So I don't see the problem. All the select...if does is make sure that > >the CEC_CORE can be reached from the HDMI driver if someone enabled the > >CEC notifier (and thus CEC_CORE). > > > >>You might as well have CEC_NOTIFIER itself select CEC_CORE, and be done > >>with it, because that's basically what this boils down to. > > > >That makes no sense. > > > >If CEC_NOTIFIER is set, then both the CEC driver and the HDMI driver have to > >select CEC_CORE to ensure the right dependency. If CEC_NOTIFIER is not set, > >then only the CEC driver has to select CEC_CORE. In that case the CEC code > >is typically either integrated into the HDMI driver or it is a standalone > >device like the USB pulse8-cec driver. > > Just to make sure you aren't waiting for me to do anything: as far as I can tell > the Kconfig above will ensure the right dependencies. If you have an example > where this fails, then let me know. I am not planning on making any changes. > Frankly, I wouldn't know what to change since AFAICT it is all working with > the above Kconfig. No, I just haven't got around to it yet - I've been busy all last week trying to work out what's been causing a USB host driver to fail for a client, and as such haven't had any chance to look at much else post merge window yet. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
