| Message ID | 20210618190905.580258-1-kieran.bingham@ideasonboard.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers |
Received: from vger.kernel.org ([23.128.96.18])
by www.linuxtv.org with esmtp (Exim 4.92)
(envelope-from <linux-media-owner@vger.kernel.org>)
id 1luJri-005HaM-EN; Fri, 18 Jun 2021 19:09:20 +0000
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S233977AbhFRTL0 (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other);
Fri, 18 Jun 2021 15:11:26 -0400
Received: from perceval.ideasonboard.com ([213.167.242.64]:50668 "EHLO
perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S232208AbhFRTLZ (ORCPT
<rfc822;linux-media@vger.kernel.org>);
Fri, 18 Jun 2021 15:11:25 -0400
Received: from Monstersaurus.local
(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])
by perceval.ideasonboard.com (Postfix) with ESMTPSA id 92B5B3F0;
Fri, 18 Jun 2021 21:09:14 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;
s=mail; t=1624043354;
bh=4XJfXPrsv+/8lp80oVWiqyRtb2RTHa3MA6/bu8ZWY18=;
h=From:To:Cc:Subject:Date:From;
b=ArMgnp0ELJ0QkS0Rc3T1qq7K3K6vVfxmz3313kNJXeQMyDbjpOUQujKKpQr/foSLI
3GIEqz34422bST/9nEWlGeWyMqQ7K/KP2cOI4Z/tdzMuXkgRCBNB8t2S89VcGZbFHL
/aE70xTN75+OXwT24eudxJk2p0bxlj/DQQmXRSQI=
From: Kieran Bingham <kieran.bingham@ideasonboard.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Kieran Bingham <kieran.bingham@ideasonboard.com>,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Subject: [PATCH] media: vsp1: Simplify DRM UIF handling
Date: Fri, 18 Jun 2021 20:09:05 +0100
Message-Id: <20210618190905.580258-1-kieran.bingham@ideasonboard.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk
List-ID: <linux-media.vger.kernel.org>
X-Mailing-List: linux-media@vger.kernel.org
X-LSpam-Score: -3.2 (---)
X-LSpam-Report: No,
score=-3.2 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_LOW=-0.7
autolearn=ham autolearn_force=no
|
| Series |
media: vsp1: Simplify DRM UIF handling
|
|
Commit Message
Kieran Bingham
June 18, 2021, 7:09 p.m. UTC
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 .... drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Comments
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); > >> }
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); }