Re: [PATCH v3 02/10] media: vsp1: drm: Don't configure hardware when the pipeline is disabled

[Ulrich Hecht <uli@fpond.eu>]

> On June 17, 2019 at 11:09 PM Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote:
> 
> 
> The vsp1_du_atomic_flush() function calls vsp1_du_pipeline_configure()
> to configure the hardware pipeline. The function is currently guaranteed
> to be called with the pipeline enabled, but this will change by future
> rework of the DU driver. Guard the hardware configuration to skip it
> when the pipeline is disabled. The hardware will be configured the next
> time the pipeline gets enabled.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 13 ++++++++++++-
>  drivers/media/platform/vsp1/vsp1_drm.h |  2 ++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
> index 7957e1439de0..900465caf1bf 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -723,6 +723,8 @@ int vsp1_du_atomic_enable(struct device *dev, unsigned int pipe_index,
>  	/* Configure all entities in the pipeline. */
>  	vsp1_du_pipeline_configure(pipe);
>  
> +	drm_pipe->enabled = true;
> +
>  unlock:
>  	mutex_unlock(&vsp1->drm->lock);
>  
> @@ -800,6 +802,8 @@ int vsp1_du_atomic_disable(struct device *dev, unsigned int pipe_index)
>  	pipe->brx->pipe = NULL;
>  	pipe->brx = NULL;
>  
> +	drm_pipe->enabled = false;
> +
>  	mutex_unlock(&vsp1->drm->lock);
>  
>  	vsp1_dlm_reset(pipe->output->dlm);
> @@ -992,7 +996,14 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
>  	}
>  
>  	vsp1_du_pipeline_setup_inputs(vsp1, pipe);
> -	vsp1_du_pipeline_configure(pipe);
> +
> +	/*
> +	 * We may get called before the pipeline gets enabled, postpone
> +	 * configuration in that case. vsp1_du_pipeline_configure() will be
> +	 * called from vsp1_du_atomic_enable().
> +	 */
> +	if (drm_pipe->enabled)
> +		vsp1_du_pipeline_configure(pipe);
>  
>  done:
>  	mutex_unlock(&vsp1->drm->lock);
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.h b/drivers/media/platform/vsp1/vsp1_drm.h
> index e85ad4366fbb..d780dafc1324 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.h
> +++ b/drivers/media/platform/vsp1/vsp1_drm.h
> @@ -20,6 +20,7 @@
>  /**
>   * vsp1_drm_pipeline - State for the API exposed to the DRM driver
>   * @pipe: the VSP1 pipeline used for display
> + * @enabled: true if the pipeline is enabled
>   * @width: output display width
>   * @height: output display height
>   * @force_brx_release: when set, release the BRx during the next reconfiguration
> @@ -31,6 +32,7 @@
>   */
>  struct vsp1_drm_pipeline {
>  	struct vsp1_pipeline pipe;
> +	bool enabled;
>  
>  	unsigned int width;
>  	unsigned int height;
> -- 
> Regards,
> 
> Laurent Pinchart
>

Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>

CU
Uli