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

[Abhinav Kumar]

On 4/19/2024 6:26 PM, Dmitry Baryshkov wrote:
> On Fri, Apr 19, 2024 at 04:43:20PM -0700, Abhinav Kumar wrote:
>>
>>
>> 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?
> 
> Please don't attach patches to the email. It makes it impossible to
> respond to them.
> 

I attached it because it was too much to paste over here.

Please review msm_framebuffer_init() in the downstream sources.

The only missing piece I can see is the handling of 
DRM_MODE_FB_MODIFIERS flags.

I am unable to trace back why this support was not present.

> Anyway, what are we missing with the current codebase? Why wasn't the
> callback / function used in the first place?
> 
>>
>> 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,
>>>
>