[PATCH v3 09/20] drm: omapdrm: Replace DSS manager state check with omapdrm CRTC state

[Tomi Valkeinen]

On 19/09/16 15:27, Laurent Pinchart wrote:
> Instead of conditioning planes update based on the DD manager state, use

s/DD/DSS/

Maybe "manager HW state" to highlight that the status is read from a HW
register.

> the enabled field newly added to the omap_crtc structure. This reduces
> the dependency from the DRM layer to the DSS layer.
> 
> The enabled field is a transitory measure, the implementation should use
> the CRTC atomic state instead. However, given that CRTCs are currently
> not enabled/disabled through their .enable() and .disable() operations
> but through a convoluted code paths starting at the associated encoder
> operations, there is not clear guarantee that the atomic state always
> matches the hardware state. This will be refactored later, at which
> point the enabled field will be removed.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes since v2:
> 
> - Use enabled field in struct omap_crtc instead of CRTC atomic state
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index cdcfda31043e..f41a638c8d65 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -40,6 +40,7 @@ struct omap_crtc {
>  
>  	bool ignore_digit_sync_lost;
>  
> +	bool enabled;
>  	bool pending;
>  	wait_queue_head_t pending_wait;
>  };
> @@ -136,6 +137,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
>  
>  	if (omap_crtc_output[channel]->output_type == OMAP_DISPLAY_TYPE_HDMI) {
>  		dispc_mgr_enable(channel, enable);
> +		omap_crtc->enabled = enable;
>  		return;
>  	}
>  
> @@ -172,6 +174,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
>  	}
>  
>  	dispc_mgr_enable(channel, enable);
> +	omap_crtc->enabled = enable;

omap_crtc_set_enabled() is only called from omap_crtc_dss_enable() and
omap_crtc_dss_disable(). It's probably a bit cleaner to set the
omap_crtc->enabled in those functions.

Btw, omap_crtc_set_enabled() has a comment:

/* Called only from the encoder enable/disable and suspend/resume
handlers. */

which doesn't seem to hold true...

 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/473f9457/attachment-0001.sig>