| Message ID | 20210808134526.119198-2-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | gpu/drm: ingenic-drm: Various improvements | expand |
Am 08.08.21 um 15:45 schrieb Paul Cercueil: > The priv->ipu_plane would get a different value further down the code, > without the first assigned value being read first; so the first > assignation can be dropped. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > index d261f7a03b18..e42eb43d8020 100644 > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > @@ -984,9 +984,6 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) > priv->dma_hwdescs->hwdesc_pal.cmd = JZ_LCD_CMD_ENABLE_PAL > | (sizeof(priv->dma_hwdescs->palette) / 4); > > - if (soc_info->has_osd) > - priv->ipu_plane = drm_plane_from_index(drm, 0); > - > primary = priv->soc_info->has_osd ? &priv->f1 : &priv->f0; > > drm_plane_helper_add(primary, &ingenic_drm_plane_helper_funcs); >
On Sun, 2021-08-08 at 19:58 +0200, Thomas Zimmermann wrote: > > Am 08.08.21 um 15:45 schrieb Paul Cercueil: > > The priv->ipu_plane would get a different value further down the code, > > without the first assigned value being read first; so the first > > assignation can be dropped. > > > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> I think this is at best an incomplete description. How is it known that this priv->ipu_plane assignment isn't necessary for any path of any failure path after this assignment and before the new assignment? > > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c [] > > @@ -984,9 +984,6 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) > > priv->dma_hwdescs->hwdesc_pal.cmd = JZ_LCD_CMD_ENABLE_PAL > > | (sizeof(priv->dma_hwdescs->palette) / 4); > > > > - if (soc_info->has_osd) > > - priv->ipu_plane = drm_plane_from_index(drm, 0); > > - > > primary = priv->soc_info->has_osd ? &priv->f1 : &priv->f0; > > > > drm_plane_helper_add(primary, &ingenic_drm_plane_helper_funcs); > > >
Hi Joe, Le dim., août 8 2021 at 11:27:34 -0700, Joe Perches <joe@perches.com> a écrit : > On Sun, 2021-08-08 at 19:58 +0200, Thomas Zimmermann wrote: >> >> Am 08.08.21 um 15:45 schrieb Paul Cercueil: >> > The priv->ipu_plane would get a different value further down the >> code, >> > without the first assigned value being read first; so the first >> > assignation can be dropped. >> > >> > Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> >> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > I think this is at best an incomplete description. > > How is it known that this priv->ipu_plane assignment isn't > necessary for any path of any failure path after this assignment > and before the new assignment? It is only used in the .atomic_begin and .atomic_check callbacks of the CRTC. These will only ever be called after the call to drm_dev_register() which happens at the end of the probe function. Cheers, -Paul >> > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > [] >> > @@ -984,9 +984,6 @@ static int ingenic_drm_bind(struct device >> *dev, bool has_components) >> > priv->dma_hwdescs->hwdesc_pal.cmd = JZ_LCD_CMD_ENABLE_PAL >> > | (sizeof(priv->dma_hwdescs->palette) / 4); >> > >> > - if (soc_info->has_osd) >> > - priv->ipu_plane = drm_plane_from_index(drm, 0); >> > - >> > primary = priv->soc_info->has_osd ? &priv->f1 : &priv->f0; >> > >> > drm_plane_helper_add(primary, &ingenic_drm_plane_helper_funcs); >> > >> > >
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c index d261f7a03b18..e42eb43d8020 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c @@ -984,9 +984,6 @@ static int ingenic_drm_bind(struct device *dev, bool has_components) priv->dma_hwdescs->hwdesc_pal.cmd = JZ_LCD_CMD_ENABLE_PAL | (sizeof(priv->dma_hwdescs->palette) / 4); - if (soc_info->has_osd) - priv->ipu_plane = drm_plane_from_index(drm, 0); - primary = priv->soc_info->has_osd ? &priv->f1 : &priv->f0; drm_plane_helper_add(primary, &ingenic_drm_plane_helper_funcs);
The priv->ipu_plane would get a different value further down the code, without the first assigned value being read first; so the first assignation can be dropped. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 3 --- 1 file changed, 3 deletions(-)