[PATCH v9 10/18] drm: bridge: samsung-dsim: Init exynos host for first DSI transfer

[Marek Szyprowski]

On 12.12.2022 16:33, Jagan Teki wrote:
> On Mon, Dec 12, 2022 at 8:52 PM Marek Szyprowski
> <m.szyprowski at samsung.com>  wrote:
>> On 12.12.2022 09:43, Marek Szyprowski wrote:
>>> On 12.12.2022 09:32, Jagan Teki wrote:
>>>> On Mon, Dec 12, 2022 at 1:56 PM Marek Szyprowski
>>>> <m.szyprowski at samsung.com>  wrote:
>>>>> Hi Jagan,
>>>>>
>>>>> On 09.12.2022 16:23, Jagan Teki wrote:
>>>>>> The existing drm panels and bridges in Exynos required host
>>>>>> initialization during the first DSI command transfer even though
>>>>>> the initialization was done before.
>>>>>>
>>>>>> This host reinitialization is handled via DSIM_STATE_REINITIALIZED
>>>>>> flag and triggers from host transfer.
>>>>>>
>>>>>> Do this exclusively for Exynos.
>>>>>>
>>>>>> Initial logic is derived from Marek Szyprowski changes.
>>>>>>
>>>>>> Signed-off-by: Marek Szyprowski<m.szyprowski at samsung.com>
>>>>>> Signed-off-by: Jagan Teki<jagan at amarulasolutions.com>
>>>>>> ---
>>>>>> Changes from v9:
>>>>>> - derived from v8
>>>>>> - added comments
>>>>>>
>>>>>>     drivers/gpu/drm/bridge/samsung-dsim.c | 15 ++++++++++++++-
>>>>>>     include/drm/bridge/samsung-dsim.h     |  5 +++--
>>>>>>     2 files changed, 17 insertions(+), 3 deletions(-)
>>>>> The following chunk is missing compared to v8:
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>>>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>>>> index 6e9ad955ebd3..6a9403cb92ae 100644
>>>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>>>> @@ -1315,7 +1315,9 @@ static int samsung_dsim_init(struct samsung_dsim
>>>>> *dsi, unsigned int flag)
>>>>>                    return 0;
>>>>>
>>>>>            samsung_dsim_reset(dsi);
>>>>> -       samsung_dsim_enable_irq(dsi);
>>>>> +
>>>>> +       if (!(dsi->state & DSIM_STATE_INITIALIZED))
>>>>> +               samsung_dsim_enable_irq(dsi);
>>>> Is this really required? does it make sure that the IRQ does not
>>>> enable twice?
>>> That's what that check does. Without the 'if (!(dsi->state &
>>> DSIM_STATE_INITIALIZED))' check, the irqs will be enabled twice (first
>>> from pre_enable, then from the first transfer), what leads to a
>>> warning from irq core.
>> I've just noticed that we also would need to clear the
>> DSIM_STATE_REINITIALIZED flag in dsim_suspend.
>>
>> However I've found that a bit simpler patch would keep the current code
>> flow for Exynos instead of this reinitialization hack. This can be
>> applied on the "[PATCH v9 09/18] drm: bridge: samsung-dsim: Add host
>> init in pre_enable" patch:
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index 0b2e52585485..acc95c61ae45 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -1291,9 +1291,11 @@ static void samsung_dsim_atomic_pre_enable(struct
>> drm_bridge *bridge,
>>
>>           dsi->state |= DSIM_STATE_ENABLED;
>>
>> -       ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>> -       if (ret)
>> -               return;
>> +       if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
>> +               ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>> +               if (ret)
>> +                       return;
>> +       }
> Sorry, I don't understand this. Does it mean Exynos doesn't need to
> init host in pre_enable? If I remember correctly even though the host
> is initialized it has to reinitialize during the first transfer - This
> is what the Exynos requirement is. Please correct or explain here.

This is a matter of enabling power regulator(s) in the right order and 
doing the host initialization in the right moment. It was never a matter 
of re-initialization. See the current code for the reference (it uses 
the same approach as my above change). I've already explained that here:

https://lore.kernel.org/all/e96197f9-948a-997e-5453-9f9d179b5f5a@samsung.com/

If you would like to see the exact proper moment of the dsi host 
initialization on the Exynos see the code here:

https://github.com/mszyprow/linux/tree/v5.18-next-20220511-dsi-rework  and patches addingmipi_dsi_host_init() to panel/bridge drivers.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20221213/4b92e1fe/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/gif
Size: 13168 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20221213/4b92e1fe/attachment-0001.gif>