Re: [PATCH 2/4] drm/i2c: tda998x: Remove obsolete drm_connector_register() call

[Brian Starkey <brian.starkey@arm.com>]

Hi Russell,

On Sat, Oct 22, 2016 at 02:40:22PM +0100, Russell King - ARM Linux wrote:
>On Wed, Oct 19, 2016 at 10:46:48AM +0100, Brian Starkey wrote:
>> Hi Jyri,
>>
>> I believe this will break mali-dp and hdlcd, unless something changed
>> while I wasn't looking. Please see this previous thread where I did
>> the same thing and then had to have it reverted: [1]
>>
>> Before removing this, we need to refactor (at least) mali-dp and hdlcd
>> to move drm_dev_register() to the end of their ->bind() callback.
>>
>> That could be done without moving drm_dev_unregister() to the start
>> of ->unbind() if you really want to nuke the drm_connector_register()
>> call, but to maintain symmetry (and introduce correctness) I was
>> putting it off until I had a chance to remove drm_vblank_cleanup()
>> from drm_dev_unregister() (because [2]).
>
>So what is the status of this - when is it going to happen?  Without
>this happening, I can't de-midlayer armada-drm, and I can't apply
>these TDA998x patches.
>
>As armada-drm stands at the moment, it can cope with the TDA998x
>driver having the drm_connector_register(), or with it eliminated.
>When armada-drm is de-midlayered without changing TDA998x, the
>drm_connector_register() call in TDA998x produces a warning:
>
>WARNING: CPU: 0 PID: 13 at lib/kobject.c:244 kobject_add_internal+0xfc/0x2d8
>kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0)
>
>I suspect that you will end up with the same problem when you move
>the drm_dev_register() call after component_bind_all() - and I think
>the answer is... you shouldn't have de-midlayered until TDA998x had
>been updated to cope with de-midlayering, iow having had the
>drm_connector_register() call removed.
>

Well technically neither hdlcd or mali-dp ever had a midlayer, but yes
it would have been nice to have never introduced this problem in the
first place, I agree.

>Given that, I don't think we can avoid breaking mali-dp and hdlcd,
>except by combining the change into a single patch, changing all three
>drivers simultaneously (and any other driver which uses TDA998x which
>has also been de-midlayered.)
>

There is a way - we can explicitly register the connectors in hdlcd
and mali-dp (drm_connector_register_all used to be exposed for that
job, afaik to work around the kind of issue we face here).

But, that would introduce some extra churn, so probably a single patch
for all three works just fine.

I couldn't spot any other drivers I think will be affected. tilcdc
isn't de-midlayered.

>So, what I would like to see is a single patch against Linus' 4.8.0
>commit fixing mali-dp, hdlcd and any other driver, together with
>removing drm_connector_register() from TDA998x.  This is so the patch
>can be shared between all interested parties without forcing everyone
>to 4.9-rc1.  Looking at the diff between 4.8 and 4.9-rc1 for
>drivers/gpu/drm/arm, that shouldn't result in any merge conflicts -
>and if you want to follow on from that with 4.9-rc1 development, you
>can always merge 4.9-rc1 on top of that commit.
>
>I'm happy to take such a patch and publish it via my git tree as part
>of the TDA998x development if that helps - but either way we need it
>shared between all parties.

OK, I will follow up to this email with that patch. I note that it
will conflict with the series you sent out yesterday (23rd October).

Hopefully that's an agreeable solution for you, and everyone else.

Thanks,
-Brian

>
>-- 
>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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel