[PATCH 2/4] drm/bridge: dw-hdmi: add cec notifier support

[Hans Verkuil]

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.

Regards,

	Hans

PS: Sorry for the empty reply earlier: I accidentally pressed 'Send' too soon :-)