| Message ID | fecde1c7b65caa0e876a2f01769289a883014712.1553032382.git-series.maxime.ripard@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Split out the formats API and move it to a common place | expand |
Hi, On Tue, 2019-03-19 at 22:57 +0100, Maxime Ripard wrote: > drm_format_num_planes() is basically a lookup in the drm_format_info table > plus an access to the num_planes field of the appropriate entry. > > Most drivers are using this function while having access to the entry > already, which means that we will perform an unnecessary lookup. Removing > the call to drm_format_num_planes is therefore more efficient. > > Some drivers will not have access to that entry in the function, but in > this case the overhead is minimal (we just have to call drm_format_info() > to perform the lookup) and we can even avoid multiple, inefficient lookups > in some places that need multiple fields from the drm_format_info > structure. Great, happy to see drm_format_num_planes getting nuked! Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Cheers, Paul > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > drivers/gpu/drm/arm/malidp_mw.c | 2 +- > drivers/gpu/drm/armada/armada_fb.c | 3 ++- > drivers/gpu/drm/drm_fourcc.c | 16 ---------------- > drivers/gpu/drm/mediatek/mtk_drm_fb.c | 6 ++++-- > drivers/gpu/drm/meson/meson_overlay.c | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 9 ++++++--- > drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 3 ++- > drivers/gpu/drm/msm/msm_fb.c | 8 ++++++-- > drivers/gpu/drm/omapdrm/omap_fb.c | 4 +++- > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 6 +++--- > drivers/gpu/drm/tegra/fb.c | 3 ++- > drivers/gpu/drm/vc4/vc4_plane.c | 2 +- > drivers/gpu/drm/zte/zx_plane.c | 4 +--- > include/drm/drm_fourcc.h | 1 - > 14 files changed, 32 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c > index 041a64dc7167..91580b7a3781 100644 > --- a/drivers/gpu/drm/arm/malidp_mw.c > +++ b/drivers/gpu/drm/arm/malidp_mw.c > @@ -153,7 +153,7 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder, > return -EINVAL; > } > > - n_planes = drm_format_num_planes(fb->format->format); > + n_planes = fb->format->num_planes; > for (i = 0; i < n_planes; i++) { > struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, i); > /* memory write buffers are never rotated */ > diff --git a/drivers/gpu/drm/armada/armada_fb.c b/drivers/gpu/drm/armada/armada_fb.c > index 058ac7d9920f..a2f6472eb482 100644 > --- a/drivers/gpu/drm/armada/armada_fb.c > +++ b/drivers/gpu/drm/armada/armada_fb.c > @@ -87,6 +87,7 @@ struct armada_framebuffer *armada_framebuffer_create(struct drm_device *dev, > struct drm_framebuffer *armada_fb_create(struct drm_device *dev, > struct drm_file *dfile, const struct drm_mode_fb_cmd2 *mode) > { > + const struct drm_format_info *info = drm_get_format_info(dev, mode); > struct armada_gem_object *obj; > struct armada_framebuffer *dfb; > int ret; > @@ -97,7 +98,7 @@ struct drm_framebuffer *armada_fb_create(struct drm_device *dev, > mode->pitches[2]); > > /* We can only handle a single plane at the moment */ > - if (drm_format_num_planes(mode->pixel_format) > 1 && > + if (info->num_planes > 1 && > (mode->handles[0] != mode->handles[1] || > mode->handles[0] != mode->handles[2])) { > ret = -EINVAL; > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > index ba7e19d4336c..22c7fa459f65 100644 > --- a/drivers/gpu/drm/drm_fourcc.c > +++ b/drivers/gpu/drm/drm_fourcc.c > @@ -306,22 +306,6 @@ drm_get_format_info(struct drm_device *dev, > EXPORT_SYMBOL(drm_get_format_info); > > /** > - * drm_format_num_planes - get the number of planes for format > - * @format: pixel format (DRM_FORMAT_*) > - * > - * Returns: > - * The number of planes used by the specified pixel format. > - */ > -int drm_format_num_planes(uint32_t format) > -{ > - const struct drm_format_info *info; > - > - info = drm_format_info(format); > - return info ? info->num_planes : 1; > -} > -EXPORT_SYMBOL(drm_format_num_planes); > - > -/** > * drm_format_plane_cpp - determine the bytes per pixel value > * @format: pixel format (DRM_FORMAT_*) > * @plane: plane index > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c b/drivers/gpu/drm/mediatek/mtk_drm_fb.c > index e20fcaef2851..68fdef8b12bd 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c > @@ -32,10 +32,11 @@ static struct drm_framebuffer *mtk_drm_framebuffer_init(struct drm_device *dev, > const struct drm_mode_fb_cmd2 *mode, > struct drm_gem_object *obj) > { > + const struct drm_format_info *info = drm_get_format_info(dev, mode); > struct drm_framebuffer *fb; > int ret; > > - if (drm_format_num_planes(mode->pixel_format) != 1) > + if (info->num_planes != 1) > return ERR_PTR(-EINVAL); > > fb = kzalloc(sizeof(*fb), GFP_KERNEL); > @@ -88,6 +89,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev, > struct drm_file *file, > const struct drm_mode_fb_cmd2 *cmd) > { > + const struct drm_format_info *info = drm_get_format_info(dev, cmd); > struct drm_framebuffer *fb; > struct drm_gem_object *gem; > unsigned int width = cmd->width; > @@ -95,7 +97,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev, > unsigned int size, bpp; > int ret; > > - if (drm_format_num_planes(cmd->pixel_format) != 1) > + if (info->num_planes != 1) > return ERR_PTR(-EINVAL); > > gem = drm_gem_object_lookup(file, cmd->handles[0]); > diff --git a/drivers/gpu/drm/meson/meson_overlay.c b/drivers/gpu/drm/meson/meson_overlay.c > index 691a9fd16b36..8ff15d01a8f9 100644 > --- a/drivers/gpu/drm/meson/meson_overlay.c > +++ b/drivers/gpu/drm/meson/meson_overlay.c > @@ -466,7 +466,7 @@ static void meson_overlay_atomic_update(struct drm_plane *plane, > } > > /* Update Canvas with buffer address */ > - priv->viu.vd1_planes = drm_format_num_planes(fb->format->format); > + priv->viu.vd1_planes = fb->format->num_planes; > > switch (priv->viu.vd1_planes) { > case 3: > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c > index 0874f0a53bf9..1aed51b49be4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c > @@ -1040,10 +1040,11 @@ int dpu_format_check_modified_format( > const struct drm_mode_fb_cmd2 *cmd, > struct drm_gem_object **bos) > { > - int ret, i, num_base_fmt_planes; > + const struct drm_format_info *info; > const struct dpu_format *fmt; > struct dpu_hw_fmt_layout layout; > uint32_t bos_total_size = 0; > + int ret, i; > > if (!msm_fmt || !cmd || !bos) { > DRM_ERROR("invalid arguments\n"); > @@ -1051,14 +1052,16 @@ int dpu_format_check_modified_format( > } > > fmt = to_dpu_format(msm_fmt); > - num_base_fmt_planes = drm_format_num_planes(fmt->base.pixel_format); > + info = drm_format_info(fmt->base.pixel_format); > + if (!info) > + return -EINVAL; > > ret = dpu_format_get_plane_sizes(fmt, cmd->width, cmd->height, > &layout, cmd->pitches); > if (ret) > return ret; > > - for (i = 0; i < num_base_fmt_planes; i++) { > + for (i = 0; i < info->num_planes; i++) { > if (!bos[i]) { > DRM_ERROR("invalid handle for plane %d\n", i); > return -EINVAL; > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c > index 6153514db04c..72ab8d89efa4 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c > @@ -127,13 +127,14 @@ uint32_t mdp5_smp_calculate(struct mdp5_smp *smp, > const struct mdp_format *format, > u32 width, bool hdecim) > { > + const struct drm_format_info *info = drm_format_info(format->base.pixel_format); > struct mdp5_kms *mdp5_kms = get_kms(smp); > int rev = mdp5_cfg_get_hw_rev(mdp5_kms->cfg); > int i, hsub, nplanes, nlines; > u32 fmt = format->base.pixel_format; > uint32_t blkcfg = 0; > > - nplanes = drm_format_num_planes(fmt); > + nplanes = info->num_planes; > hsub = drm_format_horz_chroma_subsampling(fmt); > > /* different if BWC (compressed framebuffer?) enabled: */ > diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c > index 136058978e0f..432beddafb9e 100644 > --- a/drivers/gpu/drm/msm/msm_fb.c > +++ b/drivers/gpu/drm/msm/msm_fb.c > @@ -106,9 +106,11 @@ const struct msm_format *msm_framebuffer_format(struct drm_framebuffer *fb) > struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev, > struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd) > { > + const struct drm_format_info *info = drm_get_format_info(dev, > + mode_cmd); > struct drm_gem_object *bos[4] = {0}; > struct drm_framebuffer *fb; > - int ret, i, n = drm_format_num_planes(mode_cmd->pixel_format); > + int ret, i, n = info->num_planes; > > for (i = 0; i < n; i++) { > bos[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]); > @@ -135,6 +137,8 @@ struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev, > static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev, > const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos) > { > + const struct drm_format_info *info = drm_get_format_info(dev, > + mode_cmd); > struct msm_drm_private *priv = dev->dev_private; > struct msm_kms *kms = priv->kms; > struct msm_framebuffer *msm_fb = NULL; > @@ -147,7 +151,7 @@ static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev, > dev, mode_cmd, mode_cmd->width, mode_cmd->height, > (char *)&mode_cmd->pixel_format); > > - n = drm_format_num_planes(mode_cmd->pixel_format); > + n = info->num_planes; > hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format); > vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format); > > diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c > index 4f8eb9d08f99..cfb641363a32 100644 > --- a/drivers/gpu/drm/omapdrm/omap_fb.c > +++ b/drivers/gpu/drm/omapdrm/omap_fb.c > @@ -298,7 +298,9 @@ void omap_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m) > struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev, > struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd) > { > - unsigned int num_planes = drm_format_num_planes(mode_cmd->pixel_format); > + const struct drm_format_info *info = drm_get_format_info(dev, > + mode_cmd); > + unsigned int num_planes = info->num_planes; > struct drm_gem_object *bos[4]; > struct drm_framebuffer *fb; > int i; > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > index 97438bbbe389..606d176d5d96 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > @@ -74,19 +74,19 @@ static struct drm_framebuffer * > rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, > const struct drm_mode_fb_cmd2 *mode_cmd) > { > + const struct drm_format_info *info = drm_get_format_info(dev, > + mode_cmd); > struct drm_framebuffer *fb; > struct drm_gem_object *objs[ROCKCHIP_MAX_FB_BUFFER]; > struct drm_gem_object *obj; > unsigned int hsub; > unsigned int vsub; > - int num_planes; > + int num_planes = min_t(int, info->num_planes, ROCKCHIP_MAX_FB_BUFFER); > int ret; > int i; > > hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format); > vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format); > - num_planes = min(drm_format_num_planes(mode_cmd->pixel_format), > - ROCKCHIP_MAX_FB_BUFFER); > > for (i = 0; i < num_planes; i++) { > unsigned int width = mode_cmd->width / (i ? hsub : 1); > diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c > index 0a4ce05e00ab..bc8f9afd1b5f 100644 > --- a/drivers/gpu/drm/tegra/fb.c > +++ b/drivers/gpu/drm/tegra/fb.c > @@ -131,6 +131,7 @@ struct drm_framebuffer *tegra_fb_create(struct drm_device *drm, > struct drm_file *file, > const struct drm_mode_fb_cmd2 *cmd) > { > + const struct drm_format_info *info = drm_get_format_info(dev, cmd); > unsigned int hsub, vsub, i; > struct tegra_bo *planes[4]; > struct drm_gem_object *gem; > @@ -140,7 +141,7 @@ struct drm_framebuffer *tegra_fb_create(struct drm_device *drm, > hsub = drm_format_horz_chroma_subsampling(cmd->pixel_format); > vsub = drm_format_vert_chroma_subsampling(cmd->pixel_format); > > - for (i = 0; i < drm_format_num_planes(cmd->pixel_format); i++) { > + for (i = 0; i < info->num_planes; i++) { > unsigned int width = cmd->width / (i ? hsub : 1); > unsigned int height = cmd->height / (i ? vsub : 1); > unsigned int size, bpp; > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index 1babfeca0c92..138a9ff23b70 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -537,7 +537,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane, > u32 ctl0_offset = vc4_state->dlist_count; > const struct hvs_format *format = vc4_get_hvs_format(fb->format->format); > u64 base_format_mod = fourcc_mod_broadcom_mod(fb->modifier); > - int num_planes = drm_format_num_planes(format->drm); > + int num_planes = fb->format->num_planes; > u32 h_subsample, v_subsample; > bool mix_plane_alpha; > bool covers_screen; > diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c > index 83d236fd893c..c6a8be444300 100644 > --- a/drivers/gpu/drm/zte/zx_plane.c > +++ b/drivers/gpu/drm/zte/zx_plane.c > @@ -199,7 +199,6 @@ static void zx_vl_plane_atomic_update(struct drm_plane *plane, > u32 dst_x, dst_y, dst_w, dst_h; > uint32_t format; > int fmt; > - int num_planes; > int i; > > if (!fb) > @@ -218,9 +217,8 @@ static void zx_vl_plane_atomic_update(struct drm_plane *plane, > dst_h = drm_rect_height(dst); > > /* Set up data address registers for Y, Cb and Cr planes */ > - num_planes = drm_format_num_planes(format); > paddr_reg = layer + VL_Y; > - for (i = 0; i < num_planes; i++) { > + for (i = 0; i < fb->format->num_planes; i++) { > cma_obj = drm_fb_cma_get_gem_obj(fb, i); > paddr = cma_obj->paddr + fb->offsets[i]; > paddr += src_y * fb->pitches[i]; > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h > index b3d9d88ab290..41779b327d91 100644 > --- a/include/drm/drm_fourcc.h > +++ b/include/drm/drm_fourcc.h > @@ -268,7 +268,6 @@ drm_get_format_info(struct drm_device *dev, > uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth); > uint32_t drm_driver_legacy_fb_format(struct drm_device *dev, > uint32_t bpp, uint32_t depth); > -int drm_format_num_planes(uint32_t format); > int drm_format_plane_cpp(uint32_t format, int plane); > int drm_format_horz_chroma_subsampling(uint32_t format); > int drm_format_vert_chroma_subsampling(uint32_t format);
Hi Maxime, On Tue, 19 Mar 2019 at 21:57, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > drm_format_num_planes() is basically a lookup in the drm_format_info table > plus an access to the num_planes field of the appropriate entry. > > Most drivers are using this function while having access to the entry > already, which means that we will perform an unnecessary lookup. Removing > the call to drm_format_num_planes is therefore more efficient. > > Some drivers will not have access to that entry in the function, but in > this case the overhead is minimal (we just have to call drm_format_info() > to perform the lookup) and we can even avoid multiple, inefficient lookups > in some places that need multiple fields from the drm_format_info > structure. > I'm not fan of the duplicated loop-ups either. > -int drm_format_num_planes(uint32_t format) > -{ > - const struct drm_format_info *info; > - > - info = drm_format_info(format); > - return info ? info->num_planes : 1; > -} > -EXPORT_SYMBOL(drm_format_num_planes); > - The existing users are not updated to cater for the num_planes != 0 case... Which seems non-existent scenario since all the current format descriptions have 1+ planes. Should we add a test (alike the ones in 6/20) to ensure, that no entry has 0 planes? Is it even worth it or I'm a bit too paranoid? The above comments apply to 2/20. With the name suggestions by Paul, patches 1 to 5 (incl.) are: Reviewed-by: Emil Velikov <emil.velikov@collabora.com> HTH Emil
Hi Emil, On Tue, Apr 02, 2019 at 10:43:31AM +0100, Emil Velikov wrote: > On Tue, 19 Mar 2019 at 21:57, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > drm_format_num_planes() is basically a lookup in the drm_format_info table > > plus an access to the num_planes field of the appropriate entry. > > > > Most drivers are using this function while having access to the entry > > already, which means that we will perform an unnecessary lookup. Removing > > the call to drm_format_num_planes is therefore more efficient. > > > > Some drivers will not have access to that entry in the function, but in > > this case the overhead is minimal (we just have to call drm_format_info() > > to perform the lookup) and we can even avoid multiple, inefficient lookups > > in some places that need multiple fields from the drm_format_info > > structure. > > > > I'm not fan of the duplicated loop-ups either. > > > -int drm_format_num_planes(uint32_t format) > > -{ > > - const struct drm_format_info *info; > > - > > - info = drm_format_info(format); > > - return info ? info->num_planes : 1; > > -} > > -EXPORT_SYMBOL(drm_format_num_planes); > > - > > The existing users are not updated to cater for the num_planes != 0 > case... Which seems non-existent scenario since all the current format > descriptions have 1+ planes. > Should we add a test (alike the ones in 6/20) to ensure, that no entry > has 0 planes? Is it even worth it or I'm a bit too paranoid? > > The above comments apply to 2/20. I'm not entirely sure what you mean. num_planes is returned as is in the drm_format_num_planes function and it doesn't check for the num_planes value itself. That being said, we could definitely add some more tests to check that we haven't falling into the situation you describe, since most of the drivers indeed don't check for that value themselves. But that seems pretty othorgonal to me? > With the name suggestions by Paul, patches 1 to 5 (incl.) are: > Reviewed-by: Emil Velikov <emil.velikov@collabora.com> Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Tue, 2 Apr 2019 at 15:51, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi Emil, > > On Tue, Apr 02, 2019 at 10:43:31AM +0100, Emil Velikov wrote: > > On Tue, 19 Mar 2019 at 21:57, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > drm_format_num_planes() is basically a lookup in the drm_format_info table > > > plus an access to the num_planes field of the appropriate entry. > > > > > > Most drivers are using this function while having access to the entry > > > already, which means that we will perform an unnecessary lookup. Removing > > > the call to drm_format_num_planes is therefore more efficient. > > > > > > Some drivers will not have access to that entry in the function, but in > > > this case the overhead is minimal (we just have to call drm_format_info() > > > to perform the lookup) and we can even avoid multiple, inefficient lookups > > > in some places that need multiple fields from the drm_format_info > > > structure. > > > > > > > I'm not fan of the duplicated loop-ups either. > > > > > -int drm_format_num_planes(uint32_t format) > > > -{ > > > - const struct drm_format_info *info; > > > - > > > - info = drm_format_info(format); > > > - return info ? info->num_planes : 1; > > > -} > > > -EXPORT_SYMBOL(drm_format_num_planes); > > > - > > > > The existing users are not updated to cater for the num_planes != 0 > > case... Which seems non-existent scenario since all the current format > > descriptions have 1+ planes. > > Should we add a test (alike the ones in 6/20) to ensure, that no entry > > has 0 planes? Is it even worth it or I'm a bit too paranoid? > > > > The above comments apply to 2/20. > > I'm not entirely sure what you mean. num_planes is returned as is in > the drm_format_num_planes function and it doesn't check for the > num_planes value itself. > > That being said, we could definitely add some more tests to check that > we haven't falling into the situation you describe, since most of the > drivers indeed don't check for that value themselves. But that seems > pretty othorgonal to me? > Hmm I misread the old function as "return info->num_planes ? info->num_planes : 1;" Pardon for the noise. -Emil
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c index 041a64dc7167..91580b7a3781 100644 --- a/drivers/gpu/drm/arm/malidp_mw.c +++ b/drivers/gpu/drm/arm/malidp_mw.c @@ -153,7 +153,7 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder, return -EINVAL; } - n_planes = drm_format_num_planes(fb->format->format); + n_planes = fb->format->num_planes; for (i = 0; i < n_planes; i++) { struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, i); /* memory write buffers are never rotated */ diff --git a/drivers/gpu/drm/armada/armada_fb.c b/drivers/gpu/drm/armada/armada_fb.c index 058ac7d9920f..a2f6472eb482 100644 --- a/drivers/gpu/drm/armada/armada_fb.c +++ b/drivers/gpu/drm/armada/armada_fb.c @@ -87,6 +87,7 @@ struct armada_framebuffer *armada_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *armada_fb_create(struct drm_device *dev, struct drm_file *dfile, const struct drm_mode_fb_cmd2 *mode) { + const struct drm_format_info *info = drm_get_format_info(dev, mode); struct armada_gem_object *obj; struct armada_framebuffer *dfb; int ret; @@ -97,7 +98,7 @@ struct drm_framebuffer *armada_fb_create(struct drm_device *dev, mode->pitches[2]); /* We can only handle a single plane at the moment */ - if (drm_format_num_planes(mode->pixel_format) > 1 && + if (info->num_planes > 1 && (mode->handles[0] != mode->handles[1] || mode->handles[0] != mode->handles[2])) { ret = -EINVAL; diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index ba7e19d4336c..22c7fa459f65 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -306,22 +306,6 @@ drm_get_format_info(struct drm_device *dev, EXPORT_SYMBOL(drm_get_format_info); /** - * drm_format_num_planes - get the number of planes for format - * @format: pixel format (DRM_FORMAT_*) - * - * Returns: - * The number of planes used by the specified pixel format. - */ -int drm_format_num_planes(uint32_t format) -{ - const struct drm_format_info *info; - - info = drm_format_info(format); - return info ? info->num_planes : 1; -} -EXPORT_SYMBOL(drm_format_num_planes); - -/** * drm_format_plane_cpp - determine the bytes per pixel value * @format: pixel format (DRM_FORMAT_*) * @plane: plane index diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c b/drivers/gpu/drm/mediatek/mtk_drm_fb.c index e20fcaef2851..68fdef8b12bd 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c @@ -32,10 +32,11 @@ static struct drm_framebuffer *mtk_drm_framebuffer_init(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode, struct drm_gem_object *obj) { + const struct drm_format_info *info = drm_get_format_info(dev, mode); struct drm_framebuffer *fb; int ret; - if (drm_format_num_planes(mode->pixel_format) != 1) + if (info->num_planes != 1) return ERR_PTR(-EINVAL); fb = kzalloc(sizeof(*fb), GFP_KERNEL); @@ -88,6 +89,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *cmd) { + const struct drm_format_info *info = drm_get_format_info(dev, cmd); struct drm_framebuffer *fb; struct drm_gem_object *gem; unsigned int width = cmd->width; @@ -95,7 +97,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct drm_device *dev, unsigned int size, bpp; int ret; - if (drm_format_num_planes(cmd->pixel_format) != 1) + if (info->num_planes != 1) return ERR_PTR(-EINVAL); gem = drm_gem_object_lookup(file, cmd->handles[0]); diff --git a/drivers/gpu/drm/meson/meson_overlay.c b/drivers/gpu/drm/meson/meson_overlay.c index 691a9fd16b36..8ff15d01a8f9 100644 --- a/drivers/gpu/drm/meson/meson_overlay.c +++ b/drivers/gpu/drm/meson/meson_overlay.c @@ -466,7 +466,7 @@ static void meson_overlay_atomic_update(struct drm_plane *plane, } /* Update Canvas with buffer address */ - priv->viu.vd1_planes = drm_format_num_planes(fb->format->format); + priv->viu.vd1_planes = fb->format->num_planes; switch (priv->viu.vd1_planes) { case 3: diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 0874f0a53bf9..1aed51b49be4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -1040,10 +1040,11 @@ int dpu_format_check_modified_format( const struct drm_mode_fb_cmd2 *cmd, struct drm_gem_object **bos) { - int ret, i, num_base_fmt_planes; + const struct drm_format_info *info; const struct dpu_format *fmt; struct dpu_hw_fmt_layout layout; uint32_t bos_total_size = 0; + int ret, i; if (!msm_fmt || !cmd || !bos) { DRM_ERROR("invalid arguments\n"); @@ -1051,14 +1052,16 @@ int dpu_format_check_modified_format( } fmt = to_dpu_format(msm_fmt); - num_base_fmt_planes = drm_format_num_planes(fmt->base.pixel_format); + info = drm_format_info(fmt->base.pixel_format); + if (!info) + return -EINVAL; ret = dpu_format_get_plane_sizes(fmt, cmd->width, cmd->height, &layout, cmd->pitches); if (ret) return ret; - for (i = 0; i < num_base_fmt_planes; i++) { + for (i = 0; i < info->num_planes; i++) { if (!bos[i]) { DRM_ERROR("invalid handle for plane %d\n", i); return -EINVAL; diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c index 6153514db04c..72ab8d89efa4 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c @@ -127,13 +127,14 @@ uint32_t mdp5_smp_calculate(struct mdp5_smp *smp, const struct mdp_format *format, u32 width, bool hdecim) { + const struct drm_format_info *info = drm_format_info(format->base.pixel_format); struct mdp5_kms *mdp5_kms = get_kms(smp); int rev = mdp5_cfg_get_hw_rev(mdp5_kms->cfg); int i, hsub, nplanes, nlines; u32 fmt = format->base.pixel_format; uint32_t blkcfg = 0; - nplanes = drm_format_num_planes(fmt); + nplanes = info->num_planes; hsub = drm_format_horz_chroma_subsampling(fmt); /* different if BWC (compressed framebuffer?) enabled: */ diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c index 136058978e0f..432beddafb9e 100644 --- a/drivers/gpu/drm/msm/msm_fb.c +++ b/drivers/gpu/drm/msm/msm_fb.c @@ -106,9 +106,11 @@ const struct msm_format *msm_framebuffer_format(struct drm_framebuffer *fb) struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd) { + const struct drm_format_info *info = drm_get_format_info(dev, + mode_cmd); struct drm_gem_object *bos[4] = {0}; struct drm_framebuffer *fb; - int ret, i, n = drm_format_num_planes(mode_cmd->pixel_format); + int ret, i, n = info->num_planes; for (i = 0; i < n; i++) { bos[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]); @@ -135,6 +137,8 @@ struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev, static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos) { + const struct drm_format_info *info = drm_get_format_info(dev, + mode_cmd); struct msm_drm_private *priv = dev->dev_private; struct msm_kms *kms = priv->kms; struct msm_framebuffer *msm_fb = NULL; @@ -147,7 +151,7 @@ static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev, dev, mode_cmd, mode_cmd->width, mode_cmd->height, (char *)&mode_cmd->pixel_format); - n = drm_format_num_planes(mode_cmd->pixel_format); + n = info->num_planes; hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format); vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format); diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c index 4f8eb9d08f99..cfb641363a32 100644 --- a/drivers/gpu/drm/omapdrm/omap_fb.c +++ b/drivers/gpu/drm/omapdrm/omap_fb.c @@ -298,7 +298,9 @@ void omap_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m) struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd) { - unsigned int num_planes = drm_format_num_planes(mode_cmd->pixel_format); + const struct drm_format_info *info = drm_get_format_info(dev, + mode_cmd); + unsigned int num_planes = info->num_planes; struct drm_gem_object *bos[4]; struct drm_framebuffer *fb; int i; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index 97438bbbe389..606d176d5d96 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -74,19 +74,19 @@ static struct drm_framebuffer * rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd) { + const struct drm_format_info *info = drm_get_format_info(dev, + mode_cmd); struct drm_framebuffer *fb; struct drm_gem_object *objs[ROCKCHIP_MAX_FB_BUFFER]; struct drm_gem_object *obj; unsigned int hsub; unsigned int vsub; - int num_planes; + int num_planes = min_t(int, info->num_planes, ROCKCHIP_MAX_FB_BUFFER); int ret; int i; hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format); vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format); - num_planes = min(drm_format_num_planes(mode_cmd->pixel_format), - ROCKCHIP_MAX_FB_BUFFER); for (i = 0; i < num_planes; i++) { unsigned int width = mode_cmd->width / (i ? hsub : 1); diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c index 0a4ce05e00ab..bc8f9afd1b5f 100644 --- a/drivers/gpu/drm/tegra/fb.c +++ b/drivers/gpu/drm/tegra/fb.c @@ -131,6 +131,7 @@ struct drm_framebuffer *tegra_fb_create(struct drm_device *drm, struct drm_file *file, const struct drm_mode_fb_cmd2 *cmd) { + const struct drm_format_info *info = drm_get_format_info(dev, cmd); unsigned int hsub, vsub, i; struct tegra_bo *planes[4]; struct drm_gem_object *gem; @@ -140,7 +141,7 @@ struct drm_framebuffer *tegra_fb_create(struct drm_device *drm, hsub = drm_format_horz_chroma_subsampling(cmd->pixel_format); vsub = drm_format_vert_chroma_subsampling(cmd->pixel_format); - for (i = 0; i < drm_format_num_planes(cmd->pixel_format); i++) { + for (i = 0; i < info->num_planes; i++) { unsigned int width = cmd->width / (i ? hsub : 1); unsigned int height = cmd->height / (i ? vsub : 1); unsigned int size, bpp; diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 1babfeca0c92..138a9ff23b70 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -537,7 +537,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane, u32 ctl0_offset = vc4_state->dlist_count; const struct hvs_format *format = vc4_get_hvs_format(fb->format->format); u64 base_format_mod = fourcc_mod_broadcom_mod(fb->modifier); - int num_planes = drm_format_num_planes(format->drm); + int num_planes = fb->format->num_planes; u32 h_subsample, v_subsample; bool mix_plane_alpha; bool covers_screen; diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c index 83d236fd893c..c6a8be444300 100644 --- a/drivers/gpu/drm/zte/zx_plane.c +++ b/drivers/gpu/drm/zte/zx_plane.c @@ -199,7 +199,6 @@ static void zx_vl_plane_atomic_update(struct drm_plane *plane, u32 dst_x, dst_y, dst_w, dst_h; uint32_t format; int fmt; - int num_planes; int i; if (!fb) @@ -218,9 +217,8 @@ static void zx_vl_plane_atomic_update(struct drm_plane *plane, dst_h = drm_rect_height(dst); /* Set up data address registers for Y, Cb and Cr planes */ - num_planes = drm_format_num_planes(format); paddr_reg = layer + VL_Y; - for (i = 0; i < num_planes; i++) { + for (i = 0; i < fb->format->num_planes; i++) { cma_obj = drm_fb_cma_get_gem_obj(fb, i); paddr = cma_obj->paddr + fb->offsets[i]; paddr += src_y * fb->pitches[i]; diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index b3d9d88ab290..41779b327d91 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -268,7 +268,6 @@ drm_get_format_info(struct drm_device *dev, uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth); uint32_t drm_driver_legacy_fb_format(struct drm_device *dev, uint32_t bpp, uint32_t depth); -int drm_format_num_planes(uint32_t format); int drm_format_plane_cpp(uint32_t format, int plane); int drm_format_horz_chroma_subsampling(uint32_t format); int drm_format_vert_chroma_subsampling(uint32_t format);
drm_format_num_planes() is basically a lookup in the drm_format_info table plus an access to the num_planes field of the appropriate entry. Most drivers are using this function while having access to the entry already, which means that we will perform an unnecessary lookup. Removing the call to drm_format_num_planes is therefore more efficient. Some drivers will not have access to that entry in the function, but in this case the overhead is minimal (we just have to call drm_format_info() to perform the lookup) and we can even avoid multiple, inefficient lookups in some places that need multiple fields from the drm_format_info structure. Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> --- drivers/gpu/drm/arm/malidp_mw.c | 2 +- drivers/gpu/drm/armada/armada_fb.c | 3 ++- drivers/gpu/drm/drm_fourcc.c | 16 ---------------- drivers/gpu/drm/mediatek/mtk_drm_fb.c | 6 ++++-- drivers/gpu/drm/meson/meson_overlay.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 9 ++++++--- drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 3 ++- drivers/gpu/drm/msm/msm_fb.c | 8 ++++++-- drivers/gpu/drm/omapdrm/omap_fb.c | 4 +++- drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 6 +++--- drivers/gpu/drm/tegra/fb.c | 3 ++- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- drivers/gpu/drm/zte/zx_plane.c | 4 +--- include/drm/drm_fourcc.h | 1 - 14 files changed, 32 insertions(+), 37 deletions(-)