| Message ID | 20210618190905.580258-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: vsp1: Simplify DRM UIF handling | expand |
Hi Kieran, Thank you for the patch. On Fri, Jun 18, 2021 at 08:09:05PM +0100, Kieran Bingham wrote: > From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > In commit 6732f3139380 ("media: v4l: vsp1: Fix uif null pointer access") > the handling of the UIF was over complicated, and the patch applied > before review. > > Simplify it to keep the conditionals small. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > > Another one that I had lying around in my tree for too long .... It seems to have survived bitrot quite well. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c > index 06f74d410973..0c2507dc03d6 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -455,6 +455,10 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, > dev_err(vsp1->dev, "%s: failed to setup UIF after %s\n", > __func__, BRX_NAME(pipe->brx)); > > + /* If the DRM pipe does not have a UIF there is nothing we can update. */ > + if (!drm_pipe->uif) > + return 0; > + > /* > * If the UIF is not in use schedule it for removal by setting its pipe > * pointer to NULL, vsp1_du_pipeline_configure() will remove it from the > @@ -462,9 +466,9 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, > * make sure it is present in the pipeline's list of entities if it > * wasn't already. > */ > - if (drm_pipe->uif && !use_uif) { > + if (!use_uif) { > drm_pipe->uif->pipe = NULL; > - } else if (drm_pipe->uif && !drm_pipe->uif->pipe) { > + } else if (!drm_pipe->uif->pipe) { > drm_pipe->uif->pipe = pipe; > list_add_tail(&drm_pipe->uif->list_pipe, &pipe->entities); > }
Hi Kieran, Thanks for the patch. > Subject: [PATCH] media: vsp1: Simplify DRM UIF handling > > From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > In commit 6732f3139380 ("media: v4l: vsp1: Fix uif null pointer access") > the handling of the UIF was over complicated, and the patch applied before > review. > > Simplify it to keep the conditionals small. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> I forgot this. Thanks for taking care this. Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Cheers, Biju > --- > > Another one that I had lying around in my tree for too long .... > > drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c > index 06f74d410973..0c2507dc03d6 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -455,6 +455,10 @@ static int vsp1_du_pipeline_setup_inputs(struct > vsp1_device *vsp1, > dev_err(vsp1->dev, "%s: failed to setup UIF after %s\n", > __func__, BRX_NAME(pipe->brx)); > > + /* If the DRM pipe does not have a UIF there is nothing we can > update. */ > + if (!drm_pipe->uif) > + return 0; > + > /* > * If the UIF is not in use schedule it for removal by setting its > pipe > * pointer to NULL, vsp1_du_pipeline_configure() will remove it from > the @@ -462,9 +466,9 @@ static int vsp1_du_pipeline_setup_inputs(struct > vsp1_device *vsp1, > * make sure it is present in the pipeline's list of entities if it > * wasn't already. > */ > - if (drm_pipe->uif && !use_uif) { > + if (!use_uif) { > drm_pipe->uif->pipe = NULL; > - } else if (drm_pipe->uif && !drm_pipe->uif->pipe) { > + } else if (!drm_pipe->uif->pipe) { > drm_pipe->uif->pipe = pipe; > list_add_tail(&drm_pipe->uif->list_pipe, &pipe->entities); > } > -- > 2.30.2
On 18/06/2021 20:33, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Fri, Jun 18, 2021 at 08:09:05PM +0100, Kieran Bingham wrote: >> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> In commit 6732f3139380 ("media: v4l: vsp1: Fix uif null pointer access") >> the handling of the UIF was over complicated, and the patch applied >> before review. >> >> Simplify it to keep the conditionals small. >> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> --- >> >> Another one that I had lying around in my tree for too long .... > > It seems to have survived bitrot quite well. This still applies with git am to my tree here ... > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, do I need to do anything else to progress this ? -- Kieran > >> drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c >> index 06f74d410973..0c2507dc03d6 100644 >> --- a/drivers/media/platform/vsp1/vsp1_drm.c >> +++ b/drivers/media/platform/vsp1/vsp1_drm.c >> @@ -455,6 +455,10 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, >> dev_err(vsp1->dev, "%s: failed to setup UIF after %s\n", >> __func__, BRX_NAME(pipe->brx)); >> >> + /* If the DRM pipe does not have a UIF there is nothing we can update. */ >> + if (!drm_pipe->uif) >> + return 0; >> + >> /* >> * If the UIF is not in use schedule it for removal by setting its pipe >> * pointer to NULL, vsp1_du_pipeline_configure() will remove it from the >> @@ -462,9 +466,9 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, >> * make sure it is present in the pipeline's list of entities if it >> * wasn't already. >> */ >> - if (drm_pipe->uif && !use_uif) { >> + if (!use_uif) { >> drm_pipe->uif->pipe = NULL; >> - } else if (drm_pipe->uif && !drm_pipe->uif->pipe) { >> + } else if (!drm_pipe->uif->pipe) { >> drm_pipe->uif->pipe = pipe; >> list_add_tail(&drm_pipe->uif->list_pipe, &pipe->entities); >> } >
Hi Kieran, On Wed, Sep 01, 2021 at 11:07:40PM +0100, Kieran Bingham wrote: > On 18/06/2021 20:33, Laurent Pinchart wrote: > > On Fri, Jun 18, 2021 at 08:09:05PM +0100, Kieran Bingham wrote: > >> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >> > >> In commit 6732f3139380 ("media: v4l: vsp1: Fix uif null pointer access") > >> the handling of the UIF was over complicated, and the patch applied > >> before review. > >> > >> Simplify it to keep the conditionals small. > >> > >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >> --- > >> > >> Another one that I had lying around in my tree for too long .... > > > > It seems to have survived bitrot quite well. > > This still applies with git am to my tree here ... > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks, do I need to do anything else to progress this ? You can ping me to remind me to send a pull request. Which you just did :-) > >> drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c > >> index 06f74d410973..0c2507dc03d6 100644 > >> --- a/drivers/media/platform/vsp1/vsp1_drm.c > >> +++ b/drivers/media/platform/vsp1/vsp1_drm.c > >> @@ -455,6 +455,10 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, > >> dev_err(vsp1->dev, "%s: failed to setup UIF after %s\n", > >> __func__, BRX_NAME(pipe->brx)); > >> > >> + /* If the DRM pipe does not have a UIF there is nothing we can update. */ > >> + if (!drm_pipe->uif) > >> + return 0; > >> + > >> /* > >> * If the UIF is not in use schedule it for removal by setting its pipe > >> * pointer to NULL, vsp1_du_pipeline_configure() will remove it from the > >> @@ -462,9 +466,9 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, > >> * make sure it is present in the pipeline's list of entities if it > >> * wasn't already. > >> */ > >> - if (drm_pipe->uif && !use_uif) { > >> + if (!use_uif) { > >> drm_pipe->uif->pipe = NULL; > >> - } else if (drm_pipe->uif && !drm_pipe->uif->pipe) { > >> + } else if (!drm_pipe->uif->pipe) { > >> drm_pipe->uif->pipe = pipe; > >> list_add_tail(&drm_pipe->uif->list_pipe, &pipe->entities); > >> } -- Regards, Laurent Pinchart
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 06f74d410973..0c2507dc03d6 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -455,6 +455,10 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, dev_err(vsp1->dev, "%s: failed to setup UIF after %s\n", __func__, BRX_NAME(pipe->brx)); + /* If the DRM pipe does not have a UIF there is nothing we can update. */ + if (!drm_pipe->uif) + return 0; + /* * If the UIF is not in use schedule it for removal by setting its pipe * pointer to NULL, vsp1_du_pipeline_configure() will remove it from the @@ -462,9 +466,9 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, * make sure it is present in the pipeline's list of entities if it * wasn't already. */ - if (drm_pipe->uif && !use_uif) { + if (!use_uif) { drm_pipe->uif->pipe = NULL; - } else if (drm_pipe->uif && !drm_pipe->uif->pipe) { + } else if (!drm_pipe->uif->pipe) { drm_pipe->uif->pipe = pipe; list_add_tail(&drm_pipe->uif->list_pipe, &pipe->entities); }