Re: [RFC][PATCH] drm: kirin: Fix dsi probe/attach logic
From: Andrzej Hajda
Date: Fri Sep 13 2019 - 04:47:25 EST
On 12.09.2019 16:18, Matt Redfearn wrote:
> On 12/09/2019 14:21, Andrzej Hajda wrote:
>> On 12.09.2019 04:38, John Stultz wrote:
>>> On Wed, Sep 4, 2019 at 3:26 AM Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote:
>>>> On 03.09.2019 18:18, John Stultz wrote:
>>>>> On Mon, Sep 2, 2019 at 6:22 AM Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote:
>>>>>> On 30.08.2019 19:00, Rob Clark wrote:
>>>>>>> On Thu, Aug 29, 2019 at 11:52 PM Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote:
>>>>>>>> Of course it seems you have different opinion what is the right thing in
>>>>>>>> this case, so if you convince us that your approach is better one can
>>>>>>>> revert the patch.
>>>>>>> I guess my strongest / most immediate opinion is to not break other
>>>>>>> existing adv75xx bridge users.
>>>>>> It is pity that breakage happened, and next time we should be more
>>>>>> strict about testing other platforms, before patch acceptance.
>>>>>> But reverting it now will break also platform which depend on it.
>>>>> I'm really of no opinion of which approach is better here, but I will
>>>>> say that when a patch breaks previously working boards, that's a
>>>>> regression and justifying that some other board is now enabled that
>>>>> would be broken by the revert (of a patch that is not yet upstream)
>>>>> isn't really a strong argument.
>>>>> I'm happy to work with folks to try to fixup the kirin driver if this
>>>>> patch really is the right approach, but we need someone to do the same
>>>>> for the db410c, and I don't think its fair to just dump that work onto
>>>>> folks under the threat of the board breaking.
>>>> These drivers should be fixed anyway - assumption that
>>>> drm_bridge/drm_panel will be registered before the bus it is attached to
>>>> is just incorrect.
>>>> So instead of reverting, fixing and then re-applying the patch I have
>>>> gently proposed shorter path. If you prefer long path we can try to go
>>>> this way.
>>>> Matt, is the pure revert OK for you or is it possible to prepare some
>>>> workaround allowing cooperation with both approaches?
>>> Rob/Andrzej: What's the call here?
>>> Should I resubmit the kirin fix for the adv7511 regression here?
>>> Or do we revert the adv7511 patch? I believe db410c still needs a fix.
>>> I'd just like to keep the HiKey board from breaking, so let me know so
>>> I can get the fix submitted if needed.
>> Since there is no response from Matt, we can perform revert, since there
>> are no other ideas. I will apply it tomorrow, if there are no objections.
> Sorry - yeah I think reverting is probably best at this point to avoid
> breaking in-tree platforms.
> It's a shame though that there is a built-in incompatibility within the
> tree between drivers.
Quite common in development - some issues becomes visible after some time.
> The way that the generic Synopsys DSI host driver
> probes is currently incompatible with the ADV7533 (hence the patch),
> other DSI host drivers are now compatible with the ADV7533 but break
> when we change it to probe in a similar way to panel drivers.
1. The behavior should be consistent between all hosts/device drivers.
2. DSI controlled devices can expose drm objects (drm_bridge/drm_panel)
only when they can probe, ie DSI bus they sit on must be created 1st.
1 and 2 enforces policy that all host drivers should 1st create control
bus (DSI in this case) then look for higher level objects
As a consequence all bridges and panels should 1st gather the resources
they depends on, and then they can expose higher level objects.
>> And for the future: I guess it is not possible to make adv work with old
>> and new approach, but simple workaround for adv could be added later:
>> if (source is msm or kirin)
>> ÂÂÂ do_the_old_way
>> ÂÂÂ do_correctly.
> Maybe this would work, but how do we know that the list "msm or kirin"
> is exhaustive to cope with all platforms?
By checking dts/config files.
> It seems to me the built in
> incompatibility between DSI hosts needs to be resolved really rather
> than trying to work around it in the ADV7533 driver (and any other DSI
> bus device that falls into this trap).
If you know how, please describe. Atm the only real solution I see is
explicit workaround to allow new adv7511, then fixing controllers,
together with workaround-removal.
OK, it could be possible to change msm, kirin and adv in one patch,
however I am not sure if this is the best solution.
> Anyway, my platform is out of tree so better to revert my patch and keep
> the in-tree platforms working.
> Thanks everyone.
>> And remove it after fixing both dsi masters.