Hi Laurent, On 26/02/18 21:45, Laurent Pinchart wrote: > The DRM support code manages a pipeline of VSP entities, each backed by > a media entity. When starting or stopping the pipeline, it starts and > stops the media pipeline through the media API in order to store the > pipeline pointer in every entity. > > The driver doesn't use the pipe pointer in media entities, neither does > it rely on the other effects of the media_pipeline_start() and > media_pipeline_stop() functions. Furthermore, as the media links for the > DRM pipeline are never set up correctly, and as the pipeline can be > modified dynamically when enabling or disabling planes, the current > implementation is not correct. Remove the incorrect and unneeded code. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > --- > drivers/media/platform/vsp1/vsp1_drm.c | 18 +++--------------- > 1 file changed, 3 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c > index b8fee1834253..e31fb371eaf9 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -109,8 +109,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, > if (ret == -ETIMEDOUT) > dev_err(vsp1->dev, "DRM pipeline stop timeout\n"); > > - media_pipeline_stop(&pipe->output->entity.subdev.entity); > - > for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) { > struct vsp1_rwpf *rpf = pipe->inputs[i]; > > @@ -224,11 +222,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, > } > > /* > - * Mark the pipeline as streaming and enable the VSP1. This will store > - * the pipeline pointer in all entities, which the s_stream handlers > - * will need. We don't start the entities themselves right at this point > - * as there's no plane configured yet, so we can't start processing > - * buffers. > + * Enable the VSP1. We don't start the entities themselves right at this > + * point as there's no plane configured yet, so we can't start > + * processing buffers. > */ > ret = vsp1_device_get(vsp1); > if (ret < 0) > @@ -241,14 +237,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, > drm_pipe->du_complete = cfg->callback; > drm_pipe->du_private = cfg->callback_data; > > - ret = media_pipeline_start(&pipe->output->entity.subdev.entity, > - &pipe->pipe); > - if (ret < 0) { > - dev_dbg(vsp1->dev, "%s: pipeline start failed\n", __func__); > - vsp1_device_put(vsp1); > - return ret; > - } > - > /* Disable the display interrupts. */ > vsp1_write(vsp1, VI6_DISP_IRQ_STA, 0); > vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0); >