diff mbox series

[2/2] drm/tidss: Fix sync-lost issue with two displays

Message ID 20240213-tidss-fixes-v1-2-d709e8dfa505@ideasonboard.com
State New
Headers
Series drm/tidss: Fixes for zpos and multi-display |

Commit Message

Tomi Valkeinen Feb. 13, 2024, 8:16 a.m. UTC 
  A sync lost issue can be observed with two displays, when moving a plane
from one disabled display to an another disabled display, and then
enabling the display to which the plane was moved to. The exact
requirements for this to trigger are not clear.

It looks like the issue is that the layers are left enabled in the first
display's OVR registers. Even if the corresponding VP is disabled, it
still causes an issue, as if the disabled VP and its OVR would still be
in use, leading to the same VID being used by two OVRs. However, this is
just speculation based on testing the DSS behavior.

Experimentation shows that as a workaround, we can disable all the
layers in the OVR when disabling a VP. There should be no downside to
this, as the OVR is anyway effectively disabled if its VP is disabled,
and it seems to solve the sync lost issue.

However, there may be a bigger issue in play here, related to J721e
erratum i2097 ("DSS: Disabling a Layer Connected to Overlay May Result
in Synclost During the Next Frame"). Experimentation also shows that the
OVR's CHANNELIN field has similar issue. So we may need to revisit this
when we find out more about the core issue.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
---
 drivers/gpu/drm/tidss/tidss_crtc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Aradhya Bhatia Feb. 15, 2024, 12:12 p.m. UTC | #1 
On 13/02/24 13:46, Tomi Valkeinen wrote:
> A sync lost issue can be observed with two displays, when moving a plane
> from one disabled display to an another disabled display, and then
> enabling the display to which the plane was moved to. The exact
> requirements for this to trigger are not clear.
> 
> It looks like the issue is that the layers are left enabled in the first
> display's OVR registers. Even if the corresponding VP is disabled, it
> still causes an issue, as if the disabled VP and its OVR would still be
> in use, leading to the same VID being used by two OVRs. However, this is
> just speculation based on testing the DSS behavior.
> 
> Experimentation shows that as a workaround, we can disable all the
> layers in the OVR when disabling a VP. There should be no downside to
> this, as the OVR is anyway effectively disabled if its VP is disabled,
> and it seems to solve the sync lost issue.
> 
> However, there may be a bigger issue in play here, related to J721e
> erratum i2097 ("DSS: Disabling a Layer Connected to Overlay May Result
> in Synclost During the Next Frame"). Experimentation also shows that the
> OVR's CHANNELIN field has similar issue. So we may need to revisit this
> when we find out more about the core issue.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")

Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>

> ---
>  drivers/gpu/drm/tidss/tidss_crtc.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
> index 5f838980c7a1..94f8e3178df5 100644
> --- a/drivers/gpu/drm/tidss/tidss_crtc.c
> +++ b/drivers/gpu/drm/tidss/tidss_crtc.c
> @@ -265,6 +265,16 @@ static void tidss_crtc_atomic_disable(struct drm_crtc *crtc,
>  
>  	reinit_completion(&tcrtc->framedone_completion);
>  
> +	/*
> +	 * If a layer is left enabled when the videoport is disabled, and the
> +	 * vid pipeline that was used for the layer is taken into use on
> +	 * another videoport, the DSS will report sync lost issues. Disable all
> +	 * the layers here as a work-around.
> +	 */
> +	for (u32 layer = 0; layer < tidss->feat->num_planes; layer++)
> +		dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport, layer,
> +				       false);
> +
>  	dispc_vp_disable(tidss->dispc, tcrtc->hw_videoport);
>  
>  	if (!wait_for_completion_timeout(&tcrtc->framedone_completion,
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
index 5f838980c7a1..94f8e3178df5 100644
--- a/drivers/gpu/drm/tidss/tidss_crtc.c
+++ b/drivers/gpu/drm/tidss/tidss_crtc.c
@@ -265,6 +265,16 @@  static void tidss_crtc_atomic_disable(struct drm_crtc *crtc,
 
 	reinit_completion(&tcrtc->framedone_completion);
 
+	/*
+	 * If a layer is left enabled when the videoport is disabled, and the
+	 * vid pipeline that was used for the layer is taken into use on
+	 * another videoport, the DSS will report sync lost issues. Disable all
+	 * the layers here as a work-around.
+	 */
+	for (u32 layer = 0; layer < tidss->feat->num_planes; layer++)
+		dispc_ovr_enable_layer(tidss->dispc, tcrtc->hw_videoport, layer,
+				       false);
+
 	dispc_vp_disable(tidss->dispc, tcrtc->hw_videoport);
 
 	if (!wait_for_completion_timeout(&tcrtc->framedone_completion,