[PATCH 31/32] drm: Nuke fb->bits_per_pixel
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Nov 17 18:41:55 UTC 2016
Hi Ville,
On Thursday 17 Nov 2016 20:35:46 Ville Syrjälä wrote:
> On Thu, Nov 17, 2016 at 08:14:16PM +0200, Laurent Pinchart wrote:
> > On Thursday 17 Nov 2016 18:14:30 ville.syrjala at linux.intel.com wrote:
> >> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >>
> >> Replace uses of fb->bits_per_pixel with fb->format->cpp[0]*8.
> >> Less duplicated information is a good thing.
> >>
> >> Note that I didn't put parens around the cpp*8 in the below cocci
> >> script, on account of not wanting spurious parens all over the place.
> >> Instead I did the unsafe way, and tried to look over the entire diff to
> >> spot if any dangerous expressions were produced. I didn't see any.
> >>
> >> There are some cases where previously the code did X*bpp/8, so the
> >> division happened after the multiplication. Those are now just X*cpp
> >> so the division effectively happens before the multiplication,
> >> but that is perfectly fine since bpp is always a multiple of 8.
> >
> > [snip]
> >
> >> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> ---
> >>
> >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> >> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> >> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 2 +-
> >> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> >> drivers/gpu/drm/armada/armada_crtc.c | 4 ++--
> >> drivers/gpu/drm/armada/armada_fbdev.c | 2 +-
> >> drivers/gpu/drm/ast/ast_fb.c | 2 +-
> >> drivers/gpu/drm/ast/ast_mode.c | 9 +++++----
> >> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 +-
> >> drivers/gpu/drm/cirrus/cirrus_mode.c | 2 +-
> >> drivers/gpu/drm/drm_fb_helper.c | 8 ++++----
> >> drivers/gpu/drm/drm_framebuffer.c | 2 +-
> >> drivers/gpu/drm/drm_modeset_helper.c | 3 ---
> >> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 4 ++--
> >> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 6 +++---
> >> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 ++--
> >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +-
> >> drivers/gpu/drm/exynos/exynos_mixer.c | 4 ++--
> >> drivers/gpu/drm/gma500/framebuffer.c | 2 +-
> >> drivers/gpu/drm/gma500/gma_display.c | 4 ++--
> >> drivers/gpu/drm/gma500/mdfld_intel_display.c | 6 +++---
> >> drivers/gpu/drm/gma500/oaktrail_crtc.c | 4 ++--
> >> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> >> drivers/gpu/drm/i915/intel_display.c | 11 ++++-------
> >> drivers/gpu/drm/i915/intel_fbdev.c | 6 +++---
> >> drivers/gpu/drm/mgag200/mgag200_fb.c | 2 +-
> >> drivers/gpu/drm/mgag200/mgag200_mode.c | 16 ++++++++--------
> >> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 ++--
> >> drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
> >> drivers/gpu/drm/qxl/qxl_draw.c | 2 +-
> >> drivers/gpu/drm/radeon/atombios_crtc.c | 11 ++++++-----
> >> drivers/gpu/drm/radeon/r100.c | 4 ++--
> >> drivers/gpu/drm/radeon/radeon_display.c | 6 +++---
> >> drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 14 +++++++-------
> >> drivers/gpu/drm/tegra/dc.c | 2 +-
> >> drivers/gpu/drm/tegra/drm.c | 2 +-
> >> drivers/gpu/drm/udl/udl_fb.c | 2 +-
> >> drivers/gpu/drm/virtio/virtgpu_fb.c | 2 +-
> >> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 6 +++---
> >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 --
> >> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 ++--
> >> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +-
> >> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +-
> >> include/drm/drm_framebuffer.h | 7 -------
> >> 44 files changed, 89 insertions(+), 102 deletions(-)
> >
> > [snip]
> >
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >> b/drivers/gpu/drm/drm_fb_helper.c index 755e3b6e9710..bf5a06b7c0c1
> >> 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -1162,7 +1162,7 @@ static int setcolreg(struct drm_crtc *crtc, u16
> >> red, u16 green,
> >> !fb_helper->funcs->gamma_get))
> >>
> >> return -EINVAL;
> >>
> >> - WARN_ON(fb->bits_per_pixel != 8);
> >> + WARN_ON(fb->format->cpp[0] * 8 != 8);
> >
> > Maybe just WARN_ON(fb->format->cpp[0] != 1); ?
>
> I think I had a bit of cocci to do that... Yeah, it's there, and rerunning
> it does clean this up.
>
> spatch is rather finicky and does occasionally miss entire files when
> you run it in the -dir mode. But in this case it clearly do something to
> the file, so not sure why the cleanup part didn't get done.
>
> >> fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> >
> > [snip]
> >
> >> diff --git a/drivers/gpu/drm/drm_modeset_helper.c
> >> b/drivers/gpu/drm/drm_modeset_helper.c index e5d19e5fc341..3c44409244dc
> >> 100644
> >> --- a/drivers/gpu/drm/drm_modeset_helper.c
> >> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> >> @@ -82,10 +82,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device
> >> *dev, DRM_DEBUG_KMS("non-RGB pixel format %s\n",
> >> drm_get_format_name(mode_cmd->pixel_format,
> >> &format_name));
> >> -
> >> - fb->bits_per_pixel = 0;
> >> } else {
> >> - fb->bits_per_pixel = info->cpp[0] * 8;
> >> }
> >
> > I think you can remove the whole check.
>
> Not sure I can tell the tool that. Probably easier to remove it by hand
> afterwards.
Fine with me, you can create a follow-up patch that manually fixes the left-
overs.
> >> fb->dev = dev;
> >
> > [snip]
> >
> >> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c
> >> b/drivers/gpu/drm/radeon/atombios_crtc.c index
> >> 05f4ebe31ce2..8eeffc5324b2
> >> 100644
> >> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> >> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> >> @@ -1277,7 +1277,7 @@ static int dce4_crtc_do_set_base(struct drm_crtc
> >> *crtc,
> >> /* Calculate the macrotile mode index. */
> >> tile_split_bytes = 64 << tile_split;
> >> - tileb = 8 * 8 * target_fb->bits_per_pixel / 8;
> >> + tileb = (8 * 8) * target_fb->format->cpp[0];
> >
> > The parentheses are not needed.
>
> Added by cocci. I guess I can tell it to clean that up, but have to
> double check that it won't accidentally clean up something else too
> which needs extra parens.
You can also fix it manually in a subsequent patch.
> > With these fixed,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> >> tileb = min(tile_split_bytes, tileb);
> >>
> >> for (index = 0; tileb > 64; index++)
> >
> > [snip]
--
Regards,
Laurent Pinchart
More information about the dri-devel
mailing list