All of lore.kernel.org
 help / color / mirror / Atom feed
* Enable fp16 display support for DCE8+, next try.
@ 2020-12-28 18:50 ` Mario Kleiner
  0 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2020-12-28 18:50 UTC (permalink / raw)
  To: dri-devel, amd-gfx; +Cc: alexander.deucher, nicholas.kazlauskas

Hi and happy post-christmas!

I wrote a patch 1/1 that now checks plane scaling factors against
the pixel-format specific limits in the asic specific dc_plane_cap
structures during atomic check and other appropriate places.

This should prevent things like asking for scaling on fp16 framebuffers
if the hw can't do that. Hopefully this will now allow to safely enable
fp16 scanout also on older asic's like DCE-11.0, DCE-10 and DCE-8.
Patch 2/2 enables those DCE's now for fp16.

I used some quickly hacked up of IGT test kms_plane_scaling, manually
hacking the src fb size to make sure the patch correctly accepts or
rejects atomic commits based on allowable scaling factors for rgbx/a
8 bit, 10, and fp16.

This fp16 support has been successfully tested with a Sea Islands /
DCE-8 laptop. I also confirmed that at least basic HDR signalling
over HDMI works for that DCE-8 machine with a HDR monitor. For this
i used the amdvlk driver which exposes fp16 since a while on supported
hw.

There are other bugs in DC wrt. DCE-8 though, which didn't prevent
my testing, but may be worth looking into. My DCE-8 machine scrambles
the video output picture somewhat under Vulkan (radv and admvlk) if the
output signal precision isn't 8 bpc, ie. on 6 bpc (eDP laptop panel)
and 10 bpc, 12 bpc (HDMI deep color on external HDR monitor).

Another fun thing is getting a black screen if DC is enabled on at least
Linux 5.10+ (but not if i use the classic kms code in amdgpu-kms). If
i recompile the driver with a Ubuntu kconfig for Linux 5.9, the 5.10
kernel works, and the only obvious DC related difference is that DC's
new SI / DCE-6 asic support is disabled at compile time.

Thanks,
-mario


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Enable fp16 display support for DCE8+, next try.
@ 2020-12-28 18:50 ` Mario Kleiner
  0 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2020-12-28 18:50 UTC (permalink / raw)
  To: dri-devel, amd-gfx
  Cc: alexander.deucher, mario.kleiner.de, nicholas.kazlauskas

Hi and happy post-christmas!

I wrote a patch 1/1 that now checks plane scaling factors against
the pixel-format specific limits in the asic specific dc_plane_cap
structures during atomic check and other appropriate places.

This should prevent things like asking for scaling on fp16 framebuffers
if the hw can't do that. Hopefully this will now allow to safely enable
fp16 scanout also on older asic's like DCE-11.0, DCE-10 and DCE-8.
Patch 2/2 enables those DCE's now for fp16.

I used some quickly hacked up of IGT test kms_plane_scaling, manually
hacking the src fb size to make sure the patch correctly accepts or
rejects atomic commits based on allowable scaling factors for rgbx/a
8 bit, 10, and fp16.

This fp16 support has been successfully tested with a Sea Islands /
DCE-8 laptop. I also confirmed that at least basic HDR signalling
over HDMI works for that DCE-8 machine with a HDR monitor. For this
i used the amdvlk driver which exposes fp16 since a while on supported
hw.

There are other bugs in DC wrt. DCE-8 though, which didn't prevent
my testing, but may be worth looking into. My DCE-8 machine scrambles
the video output picture somewhat under Vulkan (radv and admvlk) if the
output signal precision isn't 8 bpc, ie. on 6 bpc (eDP laptop panel)
and 10 bpc, 12 bpc (HDMI deep color on external HDR monitor).

Another fun thing is getting a black screen if DC is enabled on at least
Linux 5.10+ (but not if i use the classic kms code in amdgpu-kms). If
i recompile the driver with a Ubuntu kconfig for Linux 5.9, the 5.10
kernel works, and the only obvious DC related difference is that DC's
new SI / DCE-6 asic support is disabled at compile time.

Thanks,
-mario


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] drm/amd/display: Check plane scaling against format specific hw plane caps.
  2020-12-28 18:50 ` Mario Kleiner
@ 2020-12-28 18:50   ` Mario Kleiner
  -1 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2020-12-28 18:50 UTC (permalink / raw)
  To: dri-devel, amd-gfx; +Cc: alexander.deucher, nicholas.kazlauskas

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>
---
 .../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,
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/2] drm/amd/display: Check plane scaling against format specific hw plane caps.
@ 2020-12-28 18:50   ` Mario Kleiner
  0 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2020-12-28 18:50 UTC (permalink / raw)
  To: dri-devel, amd-gfx
  Cc: alexander.deucher, mario.kleiner.de, nicholas.kazlauskas

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>
---
 .../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,
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] drm/amd/display: Enable fp16 also on DCE-8/10/11.
  2020-12-28 18:50 ` Mario Kleiner
@ 2020-12-28 18:50   ` Mario Kleiner
  -1 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2020-12-28 18:50 UTC (permalink / raw)
  To: dri-devel, amd-gfx; +Cc: alexander.deucher, nicholas.kazlauskas

The hw supports fp16, this is not only useful for HDR,
but also for standard dynamic range displays, because
it allows to get more precise color reproduction with
about 11 - 12 bpc linear precision in the unorm range
0.0 - 1.0.

Working fp16 scanout+display (and HDR over HDMI) was
verified on a DCE-8 asic, so i assume that the more
recent DCE-10/11 will work equally well, now that
format-specific plane scaling constraints are properly
enforced, e.g., the inability of fp16 to scale on older
hw like DCE-8 to DCE-11.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 2 +-
 drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 +-
 drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
index 8ab9d6c79808..f20ed05a5050 100644
--- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
@@ -385,7 +385,7 @@ static const struct dc_plane_cap plane_cap = {
 	.pixel_format_support = {
 			.argb8888 = true,
 			.nv12 = false,
-			.fp16 = false
+			.fp16 = true
 	},
 
 	.max_upscale_factor = {
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
index 3f63822b8e28..af208f9bd03b 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
@@ -410,7 +410,7 @@ static const struct dc_plane_cap plane_cap = {
 		.pixel_format_support = {
 				.argb8888 = true,
 				.nv12 = false,
-				.fp16 = false
+				.fp16 = true
 		},
 
 		.max_upscale_factor = {
diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
index 390a0fa37239..26fe25caa281 100644
--- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
@@ -402,7 +402,7 @@ static const struct dc_plane_cap plane_cap = {
 	.pixel_format_support = {
 			.argb8888 = true,
 			.nv12 = false,
-			.fp16 = false
+			.fp16 = true
 	},
 
 	.max_upscale_factor = {
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] drm/amd/display: Enable fp16 also on DCE-8/10/11.
@ 2020-12-28 18:50   ` Mario Kleiner
  0 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2020-12-28 18:50 UTC (permalink / raw)
  To: dri-devel, amd-gfx
  Cc: alexander.deucher, mario.kleiner.de, nicholas.kazlauskas

The hw supports fp16, this is not only useful for HDR,
but also for standard dynamic range displays, because
it allows to get more precise color reproduction with
about 11 - 12 bpc linear precision in the unorm range
0.0 - 1.0.

Working fp16 scanout+display (and HDR over HDMI) was
verified on a DCE-8 asic, so i assume that the more
recent DCE-10/11 will work equally well, now that
format-specific plane scaling constraints are properly
enforced, e.g., the inability of fp16 to scale on older
hw like DCE-8 to DCE-11.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 2 +-
 drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 +-
 drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
index 8ab9d6c79808..f20ed05a5050 100644
--- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
@@ -385,7 +385,7 @@ static const struct dc_plane_cap plane_cap = {
 	.pixel_format_support = {
 			.argb8888 = true,
 			.nv12 = false,
-			.fp16 = false
+			.fp16 = true
 	},
 
 	.max_upscale_factor = {
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
index 3f63822b8e28..af208f9bd03b 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
@@ -410,7 +410,7 @@ static const struct dc_plane_cap plane_cap = {
 		.pixel_format_support = {
 				.argb8888 = true,
 				.nv12 = false,
-				.fp16 = false
+				.fp16 = true
 		},
 
 		.max_upscale_factor = {
diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
index 390a0fa37239..26fe25caa281 100644
--- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
@@ -402,7 +402,7 @@ static const struct dc_plane_cap plane_cap = {
 	.pixel_format_support = {
 			.argb8888 = true,
 			.nv12 = false,
-			.fp16 = false
+			.fp16 = true
 	},
 
 	.max_upscale_factor = {
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: Enable fp16 display support for DCE8+, next try.
  2020-12-28 18:50 ` Mario Kleiner
@ 2021-01-04 17:16   ` Alex Deucher
  -1 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-01-04 17:16 UTC (permalink / raw)
  To: Mario Kleiner
  Cc: Deucher, Alexander, amd-gfx list, Maling list - DRI developers,
	Kazlauskas, Nicholas

On Mon, Dec 28, 2020 at 1:51 PM Mario Kleiner
<mario.kleiner.de@gmail.com> wrote:
>
> Hi and happy post-christmas!
>
> I wrote a patch 1/1 that now checks plane scaling factors against
> the pixel-format specific limits in the asic specific dc_plane_cap
> structures during atomic check and other appropriate places.
>
> This should prevent things like asking for scaling on fp16 framebuffers
> if the hw can't do that. Hopefully this will now allow to safely enable
> fp16 scanout also on older asic's like DCE-11.0, DCE-10 and DCE-8.
> Patch 2/2 enables those DCE's now for fp16.
>
> I used some quickly hacked up of IGT test kms_plane_scaling, manually
> hacking the src fb size to make sure the patch correctly accepts or
> rejects atomic commits based on allowable scaling factors for rgbx/a
> 8 bit, 10, and fp16.
>
> This fp16 support has been successfully tested with a Sea Islands /
> DCE-8 laptop. I also confirmed that at least basic HDR signalling
> over HDMI works for that DCE-8 machine with a HDR monitor. For this
> i used the amdvlk driver which exposes fp16 since a while on supported
> hw.

Patches look good to me, but I'd like to get some feedback from the
display folks as well.

>
> There are other bugs in DC wrt. DCE-8 though, which didn't prevent
> my testing, but may be worth looking into. My DCE-8 machine scrambles
> the video output picture somewhat under Vulkan (radv and admvlk) if the
> output signal precision isn't 8 bpc, ie. on 6 bpc (eDP laptop panel)
> and 10 bpc, 12 bpc (HDMI deep color on external HDR monitor).
>
> Another fun thing is getting a black screen if DC is enabled on at least
> Linux 5.10+ (but not if i use the classic kms code in amdgpu-kms). If
> i recompile the driver with a Ubuntu kconfig for Linux 5.9, the 5.10
> kernel works, and the only obvious DC related difference is that DC's
> new SI / DCE-6 asic support is disabled at compile time.

Fixed here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bdeff12a96c9a5da95c8d11fefd145eb165e32a
Patch should be in stable for 5.10 as well.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Enable fp16 display support for DCE8+, next try.
@ 2021-01-04 17:16   ` Alex Deucher
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-01-04 17:16 UTC (permalink / raw)
  To: Mario Kleiner
  Cc: Deucher, Alexander, amd-gfx list, Maling list - DRI developers,
	Kazlauskas, Nicholas

On Mon, Dec 28, 2020 at 1:51 PM Mario Kleiner
<mario.kleiner.de@gmail.com> wrote:
>
> Hi and happy post-christmas!
>
> I wrote a patch 1/1 that now checks plane scaling factors against
> the pixel-format specific limits in the asic specific dc_plane_cap
> structures during atomic check and other appropriate places.
>
> This should prevent things like asking for scaling on fp16 framebuffers
> if the hw can't do that. Hopefully this will now allow to safely enable
> fp16 scanout also on older asic's like DCE-11.0, DCE-10 and DCE-8.
> Patch 2/2 enables those DCE's now for fp16.
>
> I used some quickly hacked up of IGT test kms_plane_scaling, manually
> hacking the src fb size to make sure the patch correctly accepts or
> rejects atomic commits based on allowable scaling factors for rgbx/a
> 8 bit, 10, and fp16.
>
> This fp16 support has been successfully tested with a Sea Islands /
> DCE-8 laptop. I also confirmed that at least basic HDR signalling
> over HDMI works for that DCE-8 machine with a HDR monitor. For this
> i used the amdvlk driver which exposes fp16 since a while on supported
> hw.

Patches look good to me, but I'd like to get some feedback from the
display folks as well.

>
> There are other bugs in DC wrt. DCE-8 though, which didn't prevent
> my testing, but may be worth looking into. My DCE-8 machine scrambles
> the video output picture somewhat under Vulkan (radv and admvlk) if the
> output signal precision isn't 8 bpc, ie. on 6 bpc (eDP laptop panel)
> and 10 bpc, 12 bpc (HDMI deep color on external HDR monitor).
>
> Another fun thing is getting a black screen if DC is enabled on at least
> Linux 5.10+ (but not if i use the classic kms code in amdgpu-kms). If
> i recompile the driver with a Ubuntu kconfig for Linux 5.9, the 5.10
> kernel works, and the only obvious DC related difference is that DC's
> new SI / DCE-6 asic support is disabled at compile time.

Fixed here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bdeff12a96c9a5da95c8d11fefd145eb165e32a
Patch should be in stable for 5.10 as well.

Alex
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Check plane scaling against format specific hw plane caps.
  2020-12-28 18:50   ` Mario Kleiner
@ 2021-01-04 21:16     ` Kazlauskas, Nicholas
  -1 siblings, 0 replies; 16+ messages in thread
From: Kazlauskas, Nicholas @ 2021-01-04 21:16 UTC (permalink / raw)
  To: Mario Kleiner, dri-devel, amd-gfx; +Cc: alexander.deucher

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/amd/display: Check plane scaling against format specific hw plane caps.
@ 2021-01-04 21:16     ` Kazlauskas, Nicholas
  0 siblings, 0 replies; 16+ messages in thread
From: Kazlauskas, Nicholas @ 2021-01-04 21:16 UTC (permalink / raw)
  To: Mario Kleiner, dri-devel, amd-gfx; +Cc: alexander.deucher

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,
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amd/display: Enable fp16 also on DCE-8/10/11.
  2020-12-28 18:50   ` Mario Kleiner
@ 2021-01-04 21:17     ` Kazlauskas, Nicholas
  -1 siblings, 0 replies; 16+ messages in thread
From: Kazlauskas, Nicholas @ 2021-01-04 21:17 UTC (permalink / raw)
  To: Mario Kleiner, dri-devel, amd-gfx; +Cc: alexander.deucher

On 2020-12-28 1:50 p.m., Mario Kleiner wrote:
> The hw supports fp16, this is not only useful for HDR,
> but also for standard dynamic range displays, because
> it allows to get more precise color reproduction with
> about 11 - 12 bpc linear precision in the unorm range
> 0.0 - 1.0.
> 
> Working fp16 scanout+display (and HDR over HDMI) was
> verified on a DCE-8 asic, so i assume that the more
> recent DCE-10/11 will work equally well, now that
> format-specific plane scaling constraints are properly
> enforced, e.g., the inability of fp16 to scale on older
> hw like DCE-8 to DCE-11.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

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

Regards,
Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 2 +-
>   drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 +-
>   drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c   | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> index 8ab9d6c79808..f20ed05a5050 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> @@ -385,7 +385,7 @@ static const struct dc_plane_cap plane_cap = {
>   	.pixel_format_support = {
>   			.argb8888 = true,
>   			.nv12 = false,
> -			.fp16 = false
> +			.fp16 = true
>   	},
>   
>   	.max_upscale_factor = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> index 3f63822b8e28..af208f9bd03b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> @@ -410,7 +410,7 @@ static const struct dc_plane_cap plane_cap = {
>   		.pixel_format_support = {
>   				.argb8888 = true,
>   				.nv12 = false,
> -				.fp16 = false
> +				.fp16 = true
>   		},
>   
>   		.max_upscale_factor = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> index 390a0fa37239..26fe25caa281 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> @@ -402,7 +402,7 @@ static const struct dc_plane_cap plane_cap = {
>   	.pixel_format_support = {
>   			.argb8888 = true,
>   			.nv12 = false,
> -			.fp16 = false
> +			.fp16 = true
>   	},
>   
>   	.max_upscale_factor = {
> 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amd/display: Enable fp16 also on DCE-8/10/11.
@ 2021-01-04 21:17     ` Kazlauskas, Nicholas
  0 siblings, 0 replies; 16+ messages in thread
From: Kazlauskas, Nicholas @ 2021-01-04 21:17 UTC (permalink / raw)
  To: Mario Kleiner, dri-devel, amd-gfx; +Cc: alexander.deucher

On 2020-12-28 1:50 p.m., Mario Kleiner wrote:
> The hw supports fp16, this is not only useful for HDR,
> but also for standard dynamic range displays, because
> it allows to get more precise color reproduction with
> about 11 - 12 bpc linear precision in the unorm range
> 0.0 - 1.0.
> 
> Working fp16 scanout+display (and HDR over HDMI) was
> verified on a DCE-8 asic, so i assume that the more
> recent DCE-10/11 will work equally well, now that
> format-specific plane scaling constraints are properly
> enforced, e.g., the inability of fp16 to scale on older
> hw like DCE-8 to DCE-11.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

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

Regards,
Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 2 +-
>   drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 +-
>   drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c   | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> index 8ab9d6c79808..f20ed05a5050 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> @@ -385,7 +385,7 @@ static const struct dc_plane_cap plane_cap = {
>   	.pixel_format_support = {
>   			.argb8888 = true,
>   			.nv12 = false,
> -			.fp16 = false
> +			.fp16 = true
>   	},
>   
>   	.max_upscale_factor = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> index 3f63822b8e28..af208f9bd03b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> @@ -410,7 +410,7 @@ static const struct dc_plane_cap plane_cap = {
>   		.pixel_format_support = {
>   				.argb8888 = true,
>   				.nv12 = false,
> -				.fp16 = false
> +				.fp16 = true
>   		},
>   
>   		.max_upscale_factor = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> index 390a0fa37239..26fe25caa281 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> @@ -402,7 +402,7 @@ static const struct dc_plane_cap plane_cap = {
>   	.pixel_format_support = {
>   			.argb8888 = true,
>   			.nv12 = false,
> -			.fp16 = false
> +			.fp16 = true
>   	},
>   
>   	.max_upscale_factor = {
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amd/display: Enable fp16 also on DCE-8/10/11.
  2021-01-04 21:17     ` Kazlauskas, Nicholas
@ 2021-01-04 21:31       ` Alex Deucher
  -1 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-01-04 21:31 UTC (permalink / raw)
  To: Kazlauskas, Nicholas
  Cc: Deucher, Alexander, amd-gfx list, Maling list - DRI developers

Applied the series.  Thanks!

Alex

On Mon, Jan 4, 2021 at 4:17 PM Kazlauskas, Nicholas
<nicholas.kazlauskas@amd.com> wrote:
>
> On 2020-12-28 1:50 p.m., Mario Kleiner wrote:
> > The hw supports fp16, this is not only useful for HDR,
> > but also for standard dynamic range displays, because
> > it allows to get more precise color reproduction with
> > about 11 - 12 bpc linear precision in the unorm range
> > 0.0 - 1.0.
> >
> > Working fp16 scanout+display (and HDR over HDMI) was
> > verified on a DCE-8 asic, so i assume that the more
> > recent DCE-10/11 will work equally well, now that
> > format-specific plane scaling constraints are properly
> > enforced, e.g., the inability of fp16 to scale on older
> > hw like DCE-8 to DCE-11.
> >
> > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>
> Regards,
> Nicholas Kazlauskas
>
> > ---
> >   drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 2 +-
> >   drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 +-
> >   drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c   | 2 +-
> >   3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> > index 8ab9d6c79808..f20ed05a5050 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> > @@ -385,7 +385,7 @@ static const struct dc_plane_cap plane_cap = {
> >       .pixel_format_support = {
> >                       .argb8888 = true,
> >                       .nv12 = false,
> > -                     .fp16 = false
> > +                     .fp16 = true
> >       },
> >
> >       .max_upscale_factor = {
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> > index 3f63822b8e28..af208f9bd03b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> > @@ -410,7 +410,7 @@ static const struct dc_plane_cap plane_cap = {
> >               .pixel_format_support = {
> >                               .argb8888 = true,
> >                               .nv12 = false,
> > -                             .fp16 = false
> > +                             .fp16 = true
> >               },
> >
> >               .max_upscale_factor = {
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> > index 390a0fa37239..26fe25caa281 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> > @@ -402,7 +402,7 @@ static const struct dc_plane_cap plane_cap = {
> >       .pixel_format_support = {
> >                       .argb8888 = true,
> >                       .nv12 = false,
> > -                     .fp16 = false
> > +                     .fp16 = true
> >       },
> >
> >       .max_upscale_factor = {
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amd/display: Enable fp16 also on DCE-8/10/11.
@ 2021-01-04 21:31       ` Alex Deucher
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-01-04 21:31 UTC (permalink / raw)
  To: Kazlauskas, Nicholas
  Cc: Deucher, Alexander, Mario Kleiner, amd-gfx list,
	Maling list - DRI developers

Applied the series.  Thanks!

Alex

On Mon, Jan 4, 2021 at 4:17 PM Kazlauskas, Nicholas
<nicholas.kazlauskas@amd.com> wrote:
>
> On 2020-12-28 1:50 p.m., Mario Kleiner wrote:
> > The hw supports fp16, this is not only useful for HDR,
> > but also for standard dynamic range displays, because
> > it allows to get more precise color reproduction with
> > about 11 - 12 bpc linear precision in the unorm range
> > 0.0 - 1.0.
> >
> > Working fp16 scanout+display (and HDR over HDMI) was
> > verified on a DCE-8 asic, so i assume that the more
> > recent DCE-10/11 will work equally well, now that
> > format-specific plane scaling constraints are properly
> > enforced, e.g., the inability of fp16 to scale on older
> > hw like DCE-8 to DCE-11.
> >
> > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>
> Regards,
> Nicholas Kazlauskas
>
> > ---
> >   drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 2 +-
> >   drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 +-
> >   drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c   | 2 +-
> >   3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> > index 8ab9d6c79808..f20ed05a5050 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> > @@ -385,7 +385,7 @@ static const struct dc_plane_cap plane_cap = {
> >       .pixel_format_support = {
> >                       .argb8888 = true,
> >                       .nv12 = false,
> > -                     .fp16 = false
> > +                     .fp16 = true
> >       },
> >
> >       .max_upscale_factor = {
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> > index 3f63822b8e28..af208f9bd03b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> > @@ -410,7 +410,7 @@ static const struct dc_plane_cap plane_cap = {
> >               .pixel_format_support = {
> >                               .argb8888 = true,
> >                               .nv12 = false,
> > -                             .fp16 = false
> > +                             .fp16 = true
> >               },
> >
> >               .max_upscale_factor = {
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> > index 390a0fa37239..26fe25caa281 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> > @@ -402,7 +402,7 @@ static const struct dc_plane_cap plane_cap = {
> >       .pixel_format_support = {
> >                       .argb8888 = true,
> >                       .nv12 = false,
> > -                     .fp16 = false
> > +                     .fp16 = true
> >       },
> >
> >       .max_upscale_factor = {
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Enable fp16 display support for DCE8+, next try.
  2021-01-04 17:16   ` Alex Deucher
@ 2021-01-21  6:22     ` Mario Kleiner
  -1 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2021-01-21  6:22 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Deucher, Alexander, amd-gfx list, Maling list - DRI developers,
	Kazlauskas, Nicholas

On Mon, Jan 4, 2021 at 6:16 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Mon, Dec 28, 2020 at 1:51 PM Mario Kleiner
> <mario.kleiner.de@gmail.com> wrote:
> >
> > Hi and happy post-christmas!
> >
> > I wrote a patch 1/1 that now checks plane scaling factors against
> > the pixel-format specific limits in the asic specific dc_plane_cap
> > structures during atomic check and other appropriate places.
> >
> > This should prevent things like asking for scaling on fp16 framebuffers
> > if the hw can't do that. Hopefully this will now allow to safely enable
> > fp16 scanout also on older asic's like DCE-11.0, DCE-10 and DCE-8.
> > Patch 2/2 enables those DCE's now for fp16.
> >
> > I used some quickly hacked up of IGT test kms_plane_scaling, manually
> > hacking the src fb size to make sure the patch correctly accepts or
> > rejects atomic commits based on allowable scaling factors for rgbx/a
> > 8 bit, 10, and fp16.
> >
> > This fp16 support has been successfully tested with a Sea Islands /
> > DCE-8 laptop. I also confirmed that at least basic HDR signalling
> > over HDMI works for that DCE-8 machine with a HDR monitor. For this
> > i used the amdvlk driver which exposes fp16 since a while on supported
> > hw.
>
> Patches look good to me, but I'd like to get some feedback from the
> display folks as well.
>
> >
> > There are other bugs in DC wrt. DCE-8 though, which didn't prevent
> > my testing, but may be worth looking into. My DCE-8 machine scrambles
> > the video output picture somewhat under Vulkan (radv and admvlk) if the
> > output signal precision isn't 8 bpc, ie. on 6 bpc (eDP laptop panel)
> > and 10 bpc, 12 bpc (HDMI deep color on external HDR monitor).
> >
> > Another fun thing is getting a black screen if DC is enabled on at least
> > Linux 5.10+ (but not if i use the classic kms code in amdgpu-kms). If
> > i recompile the driver with a Ubuntu kconfig for Linux 5.9, the 5.10
> > kernel works, and the only obvious DC related difference is that DC's
> > new SI / DCE-6 asic support is disabled at compile time.
>
> Fixed here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bdeff12a96c9a5da95c8d11fefd145eb165e32a
> Patch should be in stable for 5.10 as well.

Yes, in recent 5.10 stable these fix the problem I experienced.

Thanks Alex,
-mario


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Enable fp16 display support for DCE8+, next try.
@ 2021-01-21  6:22     ` Mario Kleiner
  0 siblings, 0 replies; 16+ messages in thread
From: Mario Kleiner @ 2021-01-21  6:22 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Deucher, Alexander, amd-gfx list, Maling list - DRI developers,
	Kazlauskas, Nicholas

On Mon, Jan 4, 2021 at 6:16 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Mon, Dec 28, 2020 at 1:51 PM Mario Kleiner
> <mario.kleiner.de@gmail.com> wrote:
> >
> > Hi and happy post-christmas!
> >
> > I wrote a patch 1/1 that now checks plane scaling factors against
> > the pixel-format specific limits in the asic specific dc_plane_cap
> > structures during atomic check and other appropriate places.
> >
> > This should prevent things like asking for scaling on fp16 framebuffers
> > if the hw can't do that. Hopefully this will now allow to safely enable
> > fp16 scanout also on older asic's like DCE-11.0, DCE-10 and DCE-8.
> > Patch 2/2 enables those DCE's now for fp16.
> >
> > I used some quickly hacked up of IGT test kms_plane_scaling, manually
> > hacking the src fb size to make sure the patch correctly accepts or
> > rejects atomic commits based on allowable scaling factors for rgbx/a
> > 8 bit, 10, and fp16.
> >
> > This fp16 support has been successfully tested with a Sea Islands /
> > DCE-8 laptop. I also confirmed that at least basic HDR signalling
> > over HDMI works for that DCE-8 machine with a HDR monitor. For this
> > i used the amdvlk driver which exposes fp16 since a while on supported
> > hw.
>
> Patches look good to me, but I'd like to get some feedback from the
> display folks as well.
>
> >
> > There are other bugs in DC wrt. DCE-8 though, which didn't prevent
> > my testing, but may be worth looking into. My DCE-8 machine scrambles
> > the video output picture somewhat under Vulkan (radv and admvlk) if the
> > output signal precision isn't 8 bpc, ie. on 6 bpc (eDP laptop panel)
> > and 10 bpc, 12 bpc (HDMI deep color on external HDR monitor).
> >
> > Another fun thing is getting a black screen if DC is enabled on at least
> > Linux 5.10+ (but not if i use the classic kms code in amdgpu-kms). If
> > i recompile the driver with a Ubuntu kconfig for Linux 5.9, the 5.10
> > kernel works, and the only obvious DC related difference is that DC's
> > new SI / DCE-6 asic support is disabled at compile time.
>
> Fixed here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bdeff12a96c9a5da95c8d11fefd145eb165e32a
> Patch should be in stable for 5.10 as well.

Yes, in recent 5.10 stable these fix the problem I experienced.

Thanks Alex,
-mario


>
> Alex
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-01-21  6:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-28 18:50 Enable fp16 display support for DCE8+, next try Mario Kleiner
2020-12-28 18:50 ` Mario Kleiner
2020-12-28 18:50 ` [PATCH 1/2] drm/amd/display: Check plane scaling against format specific hw plane caps Mario Kleiner
2020-12-28 18:50   ` Mario Kleiner
2021-01-04 21:16   ` Kazlauskas, Nicholas
2021-01-04 21:16     ` Kazlauskas, Nicholas
2020-12-28 18:50 ` [PATCH 2/2] drm/amd/display: Enable fp16 also on DCE-8/10/11 Mario Kleiner
2020-12-28 18:50   ` Mario Kleiner
2021-01-04 21:17   ` Kazlauskas, Nicholas
2021-01-04 21:17     ` Kazlauskas, Nicholas
2021-01-04 21:31     ` Alex Deucher
2021-01-04 21:31       ` Alex Deucher
2021-01-04 17:16 ` Enable fp16 display support for DCE8+, next try Alex Deucher
2021-01-04 17:16   ` Alex Deucher
2021-01-21  6:22   ` Mario Kleiner
2021-01-21  6:22     ` Mario Kleiner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.