Re: [PATCH 1/2] drm/amd/display: Check plane scaling against format specific hw plane caps.

["Kazlauskas, Nicholas" <nicholas.kazlauskas@amd.com>]

On 2020-12-28 1:50 p.m., Mario Kleiner wrote:
> This takes hw constraints specific to pixel formats into account,
> e.g., the inability of older hw to scale fp16 format framebuffers.
> 
> It should now allow safely to enable fp16 formats also on DCE-8,
> DCE-10, DCE-11.0
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

I think we're fine with equating all the planes as equal since we don't 
expose underlay support on DCE.

Regards,
Nicholas Kazlauskas

> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 81 +++++++++++++++++--
>   1 file changed, 73 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 2c4dbdeec46a..a3745cd8a459 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3759,10 +3759,53 @@ static const struct drm_encoder_funcs amdgpu_dm_encoder_funcs = {
>   };
>   
>   
> +static void get_min_max_dc_plane_scaling(struct drm_device *dev,
> +					 struct drm_framebuffer *fb,
> +					 int *min_downscale, int *max_upscale)
> +{
> +	struct amdgpu_device *adev = drm_to_adev(dev);
> +	struct dc *dc = adev->dm.dc;
> +	/* Caps for all supported planes are the same on DCE and DCN 1 - 3 */
> +	struct dc_plane_cap *plane_cap = &dc->caps.planes[0];
> +
> +	switch (fb->format->format) {
> +	case DRM_FORMAT_P010:
> +	case DRM_FORMAT_NV12:
> +	case DRM_FORMAT_NV21:
> +		*max_upscale = plane_cap->max_upscale_factor.nv12;
> +		*min_downscale = plane_cap->max_downscale_factor.nv12;
> +		break;
> +
> +	case DRM_FORMAT_XRGB16161616F:
> +	case DRM_FORMAT_ARGB16161616F:
> +	case DRM_FORMAT_XBGR16161616F:
> +	case DRM_FORMAT_ABGR16161616F:
> +		*max_upscale = plane_cap->max_upscale_factor.fp16;
> +		*min_downscale = plane_cap->max_downscale_factor.fp16;
> +		break;
> +
> +	default:
> +		*max_upscale = plane_cap->max_upscale_factor.argb8888;
> +		*min_downscale = plane_cap->max_downscale_factor.argb8888;
> +		break;
> +	}
> +
> +	/*
> +	 * A factor of 1 in the plane_cap means to not allow scaling, ie. use a
> +	 * scaling factor of 1.0 == 1000 units.
> +	 */
> +	if (*max_upscale == 1)
> +		*max_upscale = 1000;
> +
> +	if (*min_downscale == 1)
> +		*min_downscale = 1000;
> +}
> +
> +
>   static int fill_dc_scaling_info(const struct drm_plane_state *state,
>   				struct dc_scaling_info *scaling_info)
>   {
> -	int scale_w, scale_h;
> +	int scale_w, scale_h, min_downscale, max_upscale;
>   
>   	memset(scaling_info, 0, sizeof(*scaling_info));
>   
> @@ -3794,17 +3837,25 @@ static int fill_dc_scaling_info(const struct drm_plane_state *state,
>   	/* DRM doesn't specify clipping on destination output. */
>   	scaling_info->clip_rect = scaling_info->dst_rect;
>   
> -	/* TODO: Validate scaling per-format with DC plane caps */
> +	/* Validate scaling per-format with DC plane caps */
> +	if (state->plane && state->plane->dev && state->fb) {
> +		get_min_max_dc_plane_scaling(state->plane->dev, state->fb,
> +					     &min_downscale, &max_upscale);
> +	} else {
> +		min_downscale = 250;
> +		max_upscale = 16000;
> +	}
> +
>   	scale_w = scaling_info->dst_rect.width * 1000 /
>   		  scaling_info->src_rect.width;
>   
> -	if (scale_w < 250 || scale_w > 16000)
> +	if (scale_w < min_downscale || scale_w > max_upscale)
>   		return -EINVAL;
>   
>   	scale_h = scaling_info->dst_rect.height * 1000 /
>   		  scaling_info->src_rect.height;
>   
> -	if (scale_h < 250 || scale_h > 16000)
> +	if (scale_h < min_downscale || scale_h > max_upscale)
>   		return -EINVAL;
>   
>   	/*
> @@ -6424,12 +6475,26 @@ static void dm_plane_helper_cleanup_fb(struct drm_plane *plane,
>   static int dm_plane_helper_check_state(struct drm_plane_state *state,
>   				       struct drm_crtc_state *new_crtc_state)
>   {
> -	int max_downscale = 0;
> -	int max_upscale = INT_MAX;
> +	struct drm_framebuffer *fb = state->fb;
> +	int min_downscale, max_upscale;
> +	int min_scale = 0;
> +	int max_scale = INT_MAX;
> +
> +	/* Plane enabled? Get min/max allowed scaling factors from plane caps. */
> +	if (fb && state->crtc) {
> +		get_min_max_dc_plane_scaling(state->crtc->dev, fb,
> +					     &min_downscale, &max_upscale);
> +		/*
> +		 * Convert to drm convention: 16.16 fixed point, instead of dc's
> +		 * 1.0 == 1000. Also drm scaling is src/dst instead of dc's
> +		 * dst/src, so min_scale = 1.0 / max_upscale, etc.
> +		 */
> +		min_scale = (1000 << 16) / max_upscale;
> +		max_scale = (1000 << 16) / min_downscale;
> +	}
>   
> -	/* TODO: These should be checked against DC plane caps */
>   	return drm_atomic_helper_check_plane_state(
> -		state, new_crtc_state, max_downscale, max_upscale, true, true);
> +		state, new_crtc_state, min_scale, max_scale, true, true);
>   }
>   
>   static int dm_plane_atomic_check(struct drm_plane *plane,
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel