[PATCH v3 11/20] drm: omapdrm: Check DSS manager state in the enable/disable helpers

[Tomi Valkeinen]

On 19/09/16 15:27, Laurent Pinchart wrote:
> The omapdrm DSS manager enable/disable operations check the DSS manager
> state to avoid double enabling/disabling. Move that code to the DSS
> manager to decrease the dependency of the DRM layer to the DSS layer.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c  | 1 -
>  drivers/gpu/drm/omapdrm/dss/output.c | 6 ++++++
>  drivers/gpu/drm/omapdrm/omap_crtc.c  | 3 ---
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 535240fba671..ab150bf21dd8 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -2911,7 +2911,6 @@ bool dispc_mgr_is_enabled(enum omap_channel channel)
>  {
>  	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
>  }
> -EXPORT_SYMBOL(dispc_mgr_is_enabled);
>  
>  void dispc_wb_enable(bool enable)
>  {
> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
> index 24f859488201..f0be621895fa 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -217,12 +217,18 @@ EXPORT_SYMBOL(dss_mgr_set_lcd_config);
>  
>  int dss_mgr_enable(enum omap_channel channel)
>  {
> +	if (dispc_mgr_is_enabled(channel))
> +		return 0;
> +
>  	return dss_mgr_ops->enable(channel);
>  }
>  EXPORT_SYMBOL(dss_mgr_enable);
>  
>  void dss_mgr_disable(enum omap_channel channel)
>  {
> +	if (!dispc_mgr_is_enabled(channel))
> +		return;
> +
>  	dss_mgr_ops->disable(channel);
>  }
>  EXPORT_SYMBOL(dss_mgr_disable);
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 4b7e16786e1e..a0c26592fc69 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -141,9 +141,6 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
>  		return;
>  	}
>  
> -	if (dispc_mgr_is_enabled(channel) == enable)
> -		return;
> -
>  	if (omap_crtc->channel == OMAP_DSS_CHANNEL_DIGIT) {
>  		/*
>  		 * Digit output produces some sync lost interrupts during the
> 

With this change omap_crtc_set_enabled() will do extra work if the
output is already enabled/disabled, and, if I'm not mistaken, will do
omap_irq_wait() there which might lead to issues.

If you remove the check, then I think the driver should make sure that
omap_crtc_set_enabled() is not called if the output is already
enabled/disabled. Maybe that can be done in
omap_crtc_dss_enable/disable, using the new enabled flag.

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160920/2974e9b7/attachment-0001.sig>