[PATCH v4 11/22] drm: omapdrm: Check the CRTC software state at enable/disable time

[Tomi Valkeinen]

On 14/12/16 02:27, Laurent Pinchart wrote:
> The omapdrm DSS manager enable/disable operations check the DSS manager
> state to avoid double enabling/disabling. Check the CRTC software state
> instead to decrease the dependency of the DRM layer to the DSS layer.
> The dispc_mgr_is_enabled() function then be turned into a static
> function, but needs to be moved up in its compilation unit to avoid a
> forward declaration.
> 
> Add a WARN_ON to catch double enable or disable that should be prevented
> by the DRM core and would be a clear sign of a bug. The warning should
> eventually be removed.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes since v3:
> 
> - Replace the hardware state check with a software state check in
>   omapdrm instead of moving it to omapdss.
> - Added a WARN_ON to catch impossible situations that would be a sign of
>   a clear bug
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c   | 27 +++++++++++++--------------
>  drivers/gpu/drm/omapdrm/dss/omapdss.h |  1 -
>  drivers/gpu/drm/omapdrm/omap_crtc.c   |  6 +++---
>  3 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index c839f6456db2..5554b72cf56a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -620,6 +620,19 @@ u32 dispc_wb_get_framedone_irq(void)
>  	return DISPC_IRQ_FRAMEDONEWB;
>  }
>  
> +void dispc_mgr_enable(enum omap_channel channel, bool enable)
> +{
> +	mgr_fld_write(channel, DISPC_MGR_FLD_ENABLE, enable);
> +	/* flush posted write */
> +	mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
> +}
> +EXPORT_SYMBOL(dispc_mgr_enable);
> +
> +static bool dispc_mgr_is_enabled(enum omap_channel channel)
> +{
> +	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
> +}
> +
>  bool dispc_mgr_go_busy(enum omap_channel channel)
>  {
>  	return mgr_fld_read(channel, DISPC_MGR_FLD_GO) == 1;
> @@ -2901,20 +2914,6 @@ enum omap_dss_output_id dispc_mgr_get_supported_outputs(enum omap_channel channe
>  }
>  EXPORT_SYMBOL(dispc_mgr_get_supported_outputs);
>  
> -void dispc_mgr_enable(enum omap_channel channel, bool enable)
> -{
> -	mgr_fld_write(channel, DISPC_MGR_FLD_ENABLE, enable);
> -	/* flush posted write */
> -	mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
> -}
> -EXPORT_SYMBOL(dispc_mgr_enable);
> -
> -bool dispc_mgr_is_enabled(enum omap_channel channel)
> -{
> -	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
> -}
> -EXPORT_SYMBOL(dispc_mgr_is_enabled);
> -

dispc_mgr_is_enabled() is only used in a WARN, so we could even drop the
function, and then no moving is needed. But I'd rather leave that WARN
at least for now. It's quite good at catching bad SW. So:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen at ti.com>

 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/20161214/584be2ab/attachment-0001.sig>