| Message ID | c1f99c379343a52a4923b3bf74a9e366f4e89dcb.1505898862.git-series.kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Ping ... This patch appears to have got lost in the post. Could someone pick it up please? -- Regards Kieran On 20/09/17 10:16, Kieran Bingham wrote: > When used as part of a display pipeline, the VSP is stopped and > restarted explicitly by the DU from its suspend and resume handlers. > There is thus no need to stop or restart pipelines in the VSP suspend > and resume handlers, and doing so would cause the hardware to be > left in a misconfigured state. > > Ensure that the VSP suspend and resume handlers do not affect DRM > based pipelines. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c > index 962e4c304076..ed25ba9d551b 100644 > --- a/drivers/media/platform/vsp1/vsp1_drv.c > +++ b/drivers/media/platform/vsp1/vsp1_drv.c > @@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct device *dev) > { > struct vsp1_device *vsp1 = dev_get_drvdata(dev); > > - vsp1_pipelines_suspend(vsp1); > + /* > + * When used as part of a display pipeline, the VSP is stopped and > + * restarted explicitly by the DU > + */ > + if (!vsp1->drm) > + vsp1_pipelines_suspend(vsp1); > + > pm_runtime_force_suspend(vsp1->dev); > > return 0; > @@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct device *dev) > struct vsp1_device *vsp1 = dev_get_drvdata(dev); > > pm_runtime_force_resume(vsp1->dev); > - vsp1_pipelines_resume(vsp1); > + > + /* > + * When used as part of a display pipeline, the VSP is stopped and > + * restarted explicitly by the DU > + */ > + if (!vsp1->drm) > + vsp1_pipelines_resume(vsp1); > > return 0; > } >
Hi Kieran, Thank you for the patch. On Wednesday, 20 September 2017 12:16:54 EET Kieran Bingham wrote: > When used as part of a display pipeline, the VSP is stopped and > restarted explicitly by the DU from its suspend and resume handlers. > There is thus no need to stop or restart pipelines in the VSP suspend > and resume handlers, and doing so would cause the hardware to be > left in a misconfigured state. > > Ensure that the VSP suspend and resume handlers do not affect DRM > based pipelines. s/DRM-base/DRM-based/ > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drv.c > b/drivers/media/platform/vsp1/vsp1_drv.c index 962e4c304076..ed25ba9d551b > 100644 > --- a/drivers/media/platform/vsp1/vsp1_drv.c > +++ b/drivers/media/platform/vsp1/vsp1_drv.c > @@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct device > *dev) { > struct vsp1_device *vsp1 = dev_get_drvdata(dev); > > - vsp1_pipelines_suspend(vsp1); > + /* > + * When used as part of a display pipeline, the VSP is stopped and > + * restarted explicitly by the DU s/DU/DU./ > + */ > + if (!vsp1->drm) > + vsp1_pipelines_suspend(vsp1); > + > pm_runtime_force_suspend(vsp1->dev); > > return 0; > @@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct device > *dev) struct vsp1_device *vsp1 = dev_get_drvdata(dev); > > pm_runtime_force_resume(vsp1->dev); > - vsp1_pipelines_resume(vsp1); > + > + /* > + * When used as part of a display pipeline, the VSP is stopped and > + * restarted explicitly by the DU s/DU/DU./ Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + if (!vsp1->drm) > + vsp1_pipelines_resume(vsp1); > > return 0; > }
Hi Laurent, On 12/11/17 04:28, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Wednesday, 20 September 2017 12:16:54 EET Kieran Bingham wrote: >> When used as part of a display pipeline, the VSP is stopped and >> restarted explicitly by the DU from its suspend and resume handlers. >> There is thus no need to stop or restart pipelines in the VSP suspend >> and resume handlers, and doing so would cause the hardware to be >> left in a misconfigured state. >> >> Ensure that the VSP suspend and resume handlers do not affect DRM >> based pipelines. > > s/DRM-base/DRM-based/ -ENOMATCH > >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> --- >> drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c >> b/drivers/media/platform/vsp1/vsp1_drv.c index 962e4c304076..ed25ba9d551b >> 100644 >> --- a/drivers/media/platform/vsp1/vsp1_drv.c >> +++ b/drivers/media/platform/vsp1/vsp1_drv.c >> @@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct device >> *dev) { >> struct vsp1_device *vsp1 = dev_get_drvdata(dev); >> >> - vsp1_pipelines_suspend(vsp1); >> + /* >> + * When used as part of a display pipeline, the VSP is stopped and >> + * restarted explicitly by the DU > > s/DU/DU./ > >> + */ >> + if (!vsp1->drm) >> + vsp1_pipelines_suspend(vsp1); >> + >> pm_runtime_force_suspend(vsp1->dev); >> >> return 0; >> @@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct device >> *dev) struct vsp1_device *vsp1 = dev_get_drvdata(dev); >> >> pm_runtime_force_resume(vsp1->dev); >> - vsp1_pipelines_resume(vsp1); >> + >> + /* >> + * When used as part of a display pipeline, the VSP is stopped and >> + * restarted explicitly by the DU > > s/DU/DU./ > > Apart from that, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, I'll add the full-stops and repost a v2.1 with your RB tag. > >> + */ >> + if (!vsp1->drm) >> + vsp1_pipelines_resume(vsp1); >> >> return 0; >> } >
Hi Kieran, On Sunday, 12 November 2017 18:38:31 EET Kieran Bingham wrote: > On 12/11/17 04:28, Laurent Pinchart wrote: > > On Wednesday, 20 September 2017 12:16:54 EET Kieran Bingham wrote: > >> When used as part of a display pipeline, the VSP is stopped and > >> restarted explicitly by the DU from its suspend and resume handlers. > >> There is thus no need to stop or restart pipelines in the VSP suspend > >> and resume handlers, and doing so would cause the hardware to be > >> left in a misconfigured state. > >> > >> Ensure that the VSP suspend and resume handlers do not affect DRM > >> based pipelines. > > > > s/DRM-base/DRM-based/ > > -ENOMATCH This was of course meant to be s/DRM based/DRM-based/ :-) > >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >> --- > >> > >> drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++++++++-- > >> 1 file changed, 14 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c > >> b/drivers/media/platform/vsp1/vsp1_drv.c index 962e4c304076..ed25ba9d551b > >> 100644 > >> --- a/drivers/media/platform/vsp1/vsp1_drv.c > >> +++ b/drivers/media/platform/vsp1/vsp1_drv.c > >> @@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct > >> device *dev) { > >> > >> struct vsp1_device *vsp1 = dev_get_drvdata(dev); > >> > >> - vsp1_pipelines_suspend(vsp1); > >> + /* > >> + * When used as part of a display pipeline, the VSP is stopped and > >> + * restarted explicitly by the DU > > > > s/DU/DU./ > > > >> + */ > >> + if (!vsp1->drm) > >> + vsp1_pipelines_suspend(vsp1); > >> + > >> > >> pm_runtime_force_suspend(vsp1->dev); > >> > >> return 0; > >> > >> @@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct > >> device > >> *dev) struct vsp1_device *vsp1 = dev_get_drvdata(dev); > >> > >> pm_runtime_force_resume(vsp1->dev); > >> > >> - vsp1_pipelines_resume(vsp1); > >> + > >> + /* > >> + * When used as part of a display pipeline, the VSP is stopped and > >> + * restarted explicitly by the DU > > > > s/DU/DU./ > > > > Apart from that, > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks, > > I'll add the full-stops and repost a v2.1 with your RB tag. > > >> + */ > >> + if (!vsp1->drm) > >> + vsp1_pipelines_resume(vsp1); > >> > >> return 0; > >> > >> }
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c index 962e4c304076..ed25ba9d551b 100644 --- a/drivers/media/platform/vsp1/vsp1_drv.c +++ b/drivers/media/platform/vsp1/vsp1_drv.c @@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct device *dev) { struct vsp1_device *vsp1 = dev_get_drvdata(dev); - vsp1_pipelines_suspend(vsp1); + /* + * When used as part of a display pipeline, the VSP is stopped and + * restarted explicitly by the DU + */ + if (!vsp1->drm) + vsp1_pipelines_suspend(vsp1); + pm_runtime_force_suspend(vsp1->dev); return 0; @@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct device *dev) struct vsp1_device *vsp1 = dev_get_drvdata(dev); pm_runtime_force_resume(vsp1->dev); - vsp1_pipelines_resume(vsp1); + + /* + * When used as part of a display pipeline, the VSP is stopped and + * restarted explicitly by the DU + */ + if (!vsp1->drm) + vsp1_pipelines_resume(vsp1); return 0; }
When used as part of a display pipeline, the VSP is stopped and restarted explicitly by the DU from its suspend and resume handlers. There is thus no need to stop or restart pipelines in the VSP suspend and resume handlers, and doing so would cause the hardware to be left in a misconfigured state. Ensure that the VSP suspend and resume handlers do not affect DRM based pipelines. Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> --- drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)