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@xxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxx>
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
