Re: [PATCH v3 4/7] drm/fourcc: Pass the format_info pointer to drm_format_plane_cpp
Hi,
On Thu 16 May 19, 12:31, Maxime Ripard wrote:
> So far, the drm_format_plane_cpp function was operating on the format's
> fourcc and was doing a lookup to retrieve the drm_format_info structure and
> return the cpp.
>
> However, this is inefficient since in most cases, we will have the
> drm_format_info pointer already available so we shouldn't have to perform a
> new lookup. Some drm_fourcc functions also already operate on the
> drm_format_info pointer for that reason, so the API is quite inconsistent
> there.
>
> Let's follow the latter pattern and remove the extra lookup while being a
> bit more consistent. In order to be extra consistent, also rename that
> function to drm_format_info_plane_cpp and to a static function in the
> header to match the current policy.
I know it's out of the scope of this patch, but we really should have an
unsigned plane index wherever it's used. We're only checking for
plane >= info->num_planes but seldom for plane < 0.
Either way, this is:
Reviewed-by: Paul Kocialkowski
Cheers,
Paul
> Reviewed-by: Emil Velikov
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +++-
> drivers/gpu/drm/arm/malidp_hw.c| 3 ++-
> drivers/gpu/drm/arm/malidp_planes.c| 2 +-
> drivers/gpu/drm/drm_client.c | 3 ++-
> drivers/gpu/drm/drm_fb_helper.c| 2 +-
> drivers/gpu/drm/drm_format_helper.c| 4 ++--
> drivers/gpu/drm/drm_fourcc.c | 20
> drivers/gpu/drm/i915/intel_sprite.c| 3 ++-
> drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 3 ++-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 2 +-
> drivers/gpu/drm/msm/msm_fb.c | 2 +-
> drivers/gpu/drm/radeon/radeon_fb.c | 4 +++-
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> drivers/gpu/drm/stm/ltdc.c | 2 +-
> drivers/gpu/drm/tegra/fb.c | 2 +-
> drivers/gpu/drm/zte/zx_plane.c | 2 +-
> include/drm/drm_fourcc.h | 18 +-
> 18 files changed, 42 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> index e47609218839..06e73a343724 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> @@ -121,6 +121,8 @@ static int amdgpufb_create_pinned_object(struct
> amdgpu_fbdev *rfbdev,
>struct drm_mode_fb_cmd2 *mode_cmd,
>struct drm_gem_object **gobj_p)
> {
> + const struct drm_format_info *info = drm_get_format_info(dev,
> + mode_cmd);
> struct amdgpu_device *adev = rfbdev->adev;
> struct drm_gem_object *gobj = NULL;
> struct amdgpu_bo *abo = NULL;
> @@ -131,7 +133,7 @@ static int amdgpufb_create_pinned_object(struct
> amdgpu_fbdev *rfbdev,
> int height = mode_cmd->height;
> u32 cpp;
>
> - cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
> + cpp = drm_format_info_plane_cpp(info, 0);
>
> /* need to align pitch with crtc limits */
> mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index 8df12e9a33bb..1c9e869f4c52 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -382,7 +382,8 @@ static void malidp500_modeset(struct malidp_hw_device
> *hwdev, struct videomode *
>
> int malidp_format_get_bpp(u32 fmt)
> {
> - int bpp = drm_format_plane_cpp(fmt, 0) * 8;
> + const struct drm_format_info *info = drm_format_info(fmt);
> + int bpp = drm_format_info_plane_cpp(info, 0) * 8;
>
> if (bpp == 0) {
> switch (fmt) {
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> b/drivers/gpu/drm/arm/malidp_planes.c
> index 8f89813d08c1..361c02988375 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -227,7 +227,7 @@ bool malidp_format_mod_supported(struct drm_device *drm,
>
> if (modifier & AFBC_SPLIT) {
> if (!info->is_yuv) {
> - if (drm_format_plane_cpp(format, 0) <= 2) {
> + if (drm_format_info_plane_cpp(info, 0) <= 2) {
> DRM_DEBUG_KMS("RGB formats <= 16bpp are not
> supported with SPLIT\n");
> return false;
> }
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index f20d1dda3961..169d8eeaa662 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -243,6 +243,7 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> static struct drm_client_buffer *
> drm_client_buffer_create(struct
[PATCH v3 4/7] drm/fourcc: Pass the format_info pointer to drm_format_plane_cpp
So far, the drm_format_plane_cpp function was operating on the format's
fourcc and was doing a lookup to retrieve the drm_format_info structure and
return the cpp.
However, this is inefficient since in most cases, we will have the
drm_format_info pointer already available so we shouldn't have to perform a
new lookup. Some drm_fourcc functions also already operate on the
drm_format_info pointer for that reason, so the API is quite inconsistent
there.
Let's follow the latter pattern and remove the extra lookup while being a
bit more consistent. In order to be extra consistent, also rename that
function to drm_format_info_plane_cpp and to a static function in the
header to match the current policy.
Reviewed-by: Emil Velikov
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +++-
drivers/gpu/drm/arm/malidp_hw.c| 3 ++-
drivers/gpu/drm/arm/malidp_planes.c| 2 +-
drivers/gpu/drm/drm_client.c | 3 ++-
drivers/gpu/drm/drm_fb_helper.c| 2 +-
drivers/gpu/drm/drm_format_helper.c| 4 ++--
drivers/gpu/drm/drm_fourcc.c | 20
drivers/gpu/drm/i915/intel_sprite.c| 3 ++-
drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 3 ++-
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 2 +-
drivers/gpu/drm/msm/msm_fb.c | 2 +-
drivers/gpu/drm/radeon/radeon_fb.c | 4 +++-
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
drivers/gpu/drm/stm/ltdc.c | 2 +-
drivers/gpu/drm/tegra/fb.c | 2 +-
drivers/gpu/drm/zte/zx_plane.c | 2 +-
include/drm/drm_fourcc.h | 18 +-
18 files changed, 42 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index e47609218839..06e73a343724 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -121,6 +121,8 @@ static int amdgpufb_create_pinned_object(struct
amdgpu_fbdev *rfbdev,
struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object **gobj_p)
{
+ const struct drm_format_info *info = drm_get_format_info(dev,
+mode_cmd);
struct amdgpu_device *adev = rfbdev->adev;
struct drm_gem_object *gobj = NULL;
struct amdgpu_bo *abo = NULL;
@@ -131,7 +133,7 @@ static int amdgpufb_create_pinned_object(struct
amdgpu_fbdev *rfbdev,
int height = mode_cmd->height;
u32 cpp;
- cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
+ cpp = drm_format_info_plane_cpp(info, 0);
/* need to align pitch with crtc limits */
mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 8df12e9a33bb..1c9e869f4c52 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -382,7 +382,8 @@ static void malidp500_modeset(struct malidp_hw_device
*hwdev, struct videomode *
int malidp_format_get_bpp(u32 fmt)
{
- int bpp = drm_format_plane_cpp(fmt, 0) * 8;
+ const struct drm_format_info *info = drm_format_info(fmt);
+ int bpp = drm_format_info_plane_cpp(info, 0) * 8;
if (bpp == 0) {
switch (fmt) {
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index 8f89813d08c1..361c02988375 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -227,7 +227,7 @@ bool malidp_format_mod_supported(struct drm_device *drm,
if (modifier & AFBC_SPLIT) {
if (!info->is_yuv) {
- if (drm_format_plane_cpp(format, 0) <= 2) {
+ if (drm_format_info_plane_cpp(info, 0) <= 2) {
DRM_DEBUG_KMS("RGB formats <= 16bpp are not
supported with SPLIT\n");
return false;
}
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index f20d1dda3961..169d8eeaa662 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -243,6 +243,7 @@ static void drm_client_buffer_delete(struct
drm_client_buffer *buffer)
static struct drm_client_buffer *
drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
u32 format)
{
+ const struct drm_format_info *info = drm_format_info(format);
struct drm_mode_create_dumb dumb_args = { };
struct drm_device *dev = client->dev;
struct drm_client_buffer *buffer;
@@ -258,7 +259,7 @@ drm_client_buffer_create(struct drm_client_dev *client, u32
width, u32 height, u
dumb_args.width = width;
dumb_args.height = height;
-
