[PATCH 1/9] drm/msm/dpu: drop dpu_format_check_modified_format

Abhinav Kumar quic_abhinavk at quicinc.com
Fri Apr 19 23:43:20 UTC 2024



On 3/19/2024 6:21 AM, Dmitry Baryshkov wrote:
> The msm_kms_funcs::check_modified_format() callback is not used by the
> driver. Drop it completely.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 45 -----------------------------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 15 ----------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  1 -
>   drivers/gpu/drm/msm/msm_kms.h               |  5 ----
>   4 files changed, 66 deletions(-)
> 

I think in this case, I am leaning towards completing the implementation 
rather than dropping it as usual.

It seems its easier to just add the support to call this like the 
attached patch?

WDYT?

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> index e366ab134249..ff0df478c958 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> @@ -960,51 +960,6 @@ int dpu_format_populate_layout(
>   	return ret;
>   }
>   
> -int dpu_format_check_modified_format(
> -		const struct msm_kms *kms,
> -		const struct msm_format *msm_fmt,
> -		const struct drm_mode_fb_cmd2 *cmd,
> -		struct drm_gem_object **bos)
> -{
> -	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");
> -		return -EINVAL;
> -	}
> -
> -	fmt = to_dpu_format(msm_fmt);
> -	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 < info->num_planes; i++) {
> -		if (!bos[i]) {
> -			DRM_ERROR("invalid handle for plane %d\n", i);
> -			return -EINVAL;
> -		}
> -		if ((i == 0) || (bos[i] != bos[0]))
> -			bos_total_size += bos[i]->size;
> -	}
> -
> -	if (bos_total_size < layout.total_size) {
> -		DRM_ERROR("buffers total size too small %u expected %u\n",
> -				bos_total_size, layout.total_size);
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
>   const struct dpu_format *dpu_get_dpu_format_ext(
>   		const uint32_t format,
>   		const uint64_t modifier)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
> index 84b8b3289f18..9442445f1a86 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
> @@ -54,21 +54,6 @@ const struct msm_format *dpu_get_msm_format(
>   		const uint32_t format,
>   		const uint64_t modifiers);
>   
> -/**
> - * dpu_format_check_modified_format - validate format and buffers for
> - *                   dpu non-standard, i.e. modified format
> - * @kms:             kms driver
> - * @msm_fmt:         pointer to the msm_fmt base pointer of an dpu_format
> - * @cmd:             fb_cmd2 structure user request
> - * @bos:             gem buffer object list
> - *
> - * Return: error code on failure, 0 on success
> - */
> -int dpu_format_check_modified_format(
> -		const struct msm_kms *kms,
> -		const struct msm_format *msm_fmt,
> -		const struct drm_mode_fb_cmd2 *cmd,
> -		struct drm_gem_object **bos);
>   
>   /**
>    * dpu_format_populate_layout - populate the given format layout based on
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index a1f5d7c4ab91..7257ac4020d8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -969,7 +969,6 @@ static const struct msm_kms_funcs kms_funcs = {
>   	.complete_commit = dpu_kms_complete_commit,
>   	.enable_vblank   = dpu_kms_enable_vblank,
>   	.disable_vblank  = dpu_kms_disable_vblank,
> -	.check_modified_format = dpu_format_check_modified_format,
>   	.get_format      = dpu_get_msm_format,
>   	.destroy         = dpu_kms_destroy,
>   	.snapshot        = dpu_kms_mdp_snapshot,
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 0641f6111b93..b794ed918b56 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -96,11 +96,6 @@ struct msm_kms_funcs {
>   	const struct msm_format *(*get_format)(struct msm_kms *kms,
>   					const uint32_t format,
>   					const uint64_t modifiers);
> -	/* do format checking on format modified through fb_cmd2 modifiers */
> -	int (*check_modified_format)(const struct msm_kms *kms,
> -			const struct msm_format *msm_fmt,
> -			const struct drm_mode_fb_cmd2 *cmd,
> -			struct drm_gem_object **bos);
>   
>   	/* misc: */
>   	long (*round_pixclk)(struct msm_kms *kms, unsigned long rate,
> 
-------------- next part --------------
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index 80166f702a0d..1ebf1699ba94 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -175,6 +175,7 @@ static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev,
 	struct drm_framebuffer *fb;
 	const struct msm_format *format;
 	int ret, i, n;
+	bool is_modified = false;
 
 	drm_dbg_state(dev, "create framebuffer: mode_cmd=%p (%dx%d@%4.4s)\n",
 			mode_cmd, mode_cmd->width, mode_cmd->height,
@@ -200,28 +201,52 @@ static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev,
 
 	msm_fb->format = format;
 
+	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
+		for (i = 0; i < ARRAY_SIZE(mode_cmd->modifier); i++) {
+			if (mode_cmd->modifier[i]) {
+				is_modified = true;
+				break;
+			}
+		}
+	}
+
 	if (n > ARRAY_SIZE(fb->obj)) {
 		ret = -EINVAL;
 		goto fail;
 	}
 
-	for (i = 0; i < n; i++) {
-		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
-		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
-		unsigned int min_size;
-
-		min_size = (height - 1) * mode_cmd->pitches[i]
-			 + width * info->cpp[i]
-			 + mode_cmd->offsets[i];
-
-		if (bos[i]->size < min_size) {
+	if (is_modified) {
+		if (!kms->funcs->check_modified_format) {
+			DRM_DEV_ERROR(dev->dev, "can't check modified fb format\n");
 			ret = -EINVAL;
 			goto fail;
+		} else {
+			ret = kms->funcs->check_modified_format(
+					kms, msm_fb->format, mode_cmd, bos);
+			if (ret)
+				goto fail;
 		}
+	} else {
+		for (i = 0; i < n; i++) {
+			unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
+			unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
+			unsigned int min_size;
 
-		msm_fb->base.obj[i] = bos[i];
+			min_size = (height - 1) * mode_cmd->pitches[i]
+				+ width * info->cpp[i]
+				+ mode_cmd->offsets[i];
+
+			if (bos[i]->size < min_size) {
+				ret = -EINVAL;
+				goto fail;
+			}
+
+		}
 	}
 
+	for (i = 0; i < n; i++)
+		msm_fb->base.obj[i] = bos[i];
+
 	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
 
 	ret = drm_framebuffer_init(dev, fb, &msm_framebuffer_funcs);


More information about the Freedreno mailing list