On 13.06.2022 13:34, Jagan Teki wrote:
> On Mon, Jun 13, 2022 at 5:02 PM Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx> wrote:
>> On 13.06.2022 13:17, Jagan Teki wrote:
>>> On Wed, May 11, 2022 at 4:01 PM Andrzej Hajda <andrzej.hajda@xxxxxxxxx> wrote:
>>>> On 04.05.2022 13:40, Jagan Teki wrote:
>>>>> Fixing up the mode flags is required in order to correlate the
>>>>> correct sync flags of the surrounding components in the chain
>>>>> to make sure the whole pipeline can work properly.
>>>>>
>>>>> So, handle the mode flags via bridge, atomic_check.
>>>>>
>>>>> v2:
>>>>> * none
>>>>>
>>>>> v1:
>>>>> * fix mode flags in atomic_check instead of mode_fixup
>>>>>
>>>>> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
>>>>> ---
>>>>> drivers/gpu/drm/bridge/samsung-dsim.c | 14 ++++++++++++++
>>>>> 1 file changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
>>>>> index b618e52d0ee3..bd78cef890e4 100644
>>>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>>>> @@ -1340,6 +1340,19 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
>>>>> pm_runtime_put_sync(dsi->dev);
>>>>> }
>>>>>
>>>>> +static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
>>>>> + struct drm_bridge_state *bridge_state,
>>>>> + struct drm_crtc_state *crtc_state,
>>>>> + struct drm_connector_state *conn_state)
>>>>> +{
>>>>> + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
>>>>> +
>>>>> + adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
>>>>> + adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
>>>> 1. Shouldn't this be in mode_fixup callback?
>>> Possible to do it on mode_fixup, yes. if we want to do the same stuff
>>> on atomic API then atomic_check is the proper helper.
>>>
>>>> 2. Where this requirement comes from? As Marek says it breaks Samsung
>>>> platforms and is against DSIM specification[1]:
>>> At least the bridge chain starting from LCDIF+DSIM requires active high sync.
>> Then please make this specific to the imx variant. The current version
>> breaks DSI operation on all Exynos based boards.
> But exynos trm also says the same?
>
> "45.2.2.1.2 RGB Interface
> Vsync, Hsync, and VDEN are active high signals"
Right, but You are forcing the negative sync signals:
adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
