Re: [RFC][PATCH] drm: kirin: Fix dsi probe/attach logic

[Andrzej Hajda <a.hajda@samsung.com>]

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@samsung.com> wrote:
>>>> On 03.09.2019 18:18, John Stultz wrote:
>>>>> On Mon, Sep 2, 2019 at 6:22 AM Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>>>> On 30.08.2019 19:00, Rob Clark wrote:
>>>>>>> On Thu, Aug 29, 2019 at 11:52 PM Andrzej Hajda <a.hajda@samsung.com> 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.
> Hi,
>
> 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
(drm_bridge/drm_panel).

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
>>
>> else
>>
>>      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.


Regards

Andrzej


>
> Anyway, my platform is out of tree so better to revert my patch and keep 
> the in-tree platforms working.
>
> Thanks everyone.
> Matt
>
>>
>> And remove it after fixing both dsi masters.
>>
>>
>> Regards
>>
>> Andrzej
>>
>>
>>> thanks
>>> -john
>>>
>>>