Re: [PATCH v1 04/14] drm/msm/dp: correct configure Colorimetry Indicator Field at MISC0

From: Dmitry Baryshkov
Date: Mon Jan 23 2023 - 17:41:30 EST


On 23/01/2023 20:24, Kuogee Hsieh wrote:
MSA MISC0 bit 1 to 7 contains Colorimetry Indicator Field. At current
implementation, Colorimetry Indicator Field of MISC0 is not configured
correctly. This patch add support of RGB formats Colorimetry.

Any Fixes tag? Not to mention that fixes should come first.


Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 5 +++--
drivers/gpu/drm/msm/dp/dp_link.c | 29 +++++++++++++++++++------
drivers/gpu/drm/msm/dp/dp_panel.c | 45 +++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
4 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 959a78c..d0d1848 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2012-2023, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
*/
#define pr_fmt(fmt) "[drm-dp] %s: " fmt, __func__
@@ -169,7 +170,7 @@ static void dp_ctrl_configure_source_params(struct dp_ctrl_private *ctrl)
tb = dp_link_get_test_bits_depth(ctrl->link,
ctrl->panel->dp_mode.bpp);
- cc = dp_link_get_colorimetry_config(ctrl->link);
+ cc = dp_panel_get_misc_colorimetry_val(ctrl->panel);
dp_catalog_ctrl_config_misc(ctrl->catalog, cc, tb);
dp_panel_timing_cfg(ctrl->panel);
}
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index f1f1d64..e957948 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2012-2023, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
*/
#define pr_fmt(fmt) "[drm-dp] %s: " fmt, __func__
@@ -12,6 +13,12 @@
#define DP_TEST_REQUEST_MASK 0x7F
+enum dynamic_range {
+ DP_DYNAMIC_RANGE_RGB_VESA = 0x00,
+ DP_DYNAMIC_RANGE_RGB_CEA = 0x01,
+ DP_DYNAMIC_RANGE_UNKNOWN = 0xFFFFFFFF,

No need to assign values here, unless they serve some purpose. Do they?

+};
+
enum audio_sample_rate {
AUDIO_SAMPLE_RATE_32_KHZ = 0x00,
AUDIO_SAMPLE_RATE_44_1_KHZ = 0x01,
@@ -1079,6 +1086,7 @@ int dp_link_process_request(struct dp_link *dp_link)
int dp_link_get_colorimetry_config(struct dp_link *dp_link)
{
u32 cc;
+ enum dynamic_range dr;
struct dp_link_private *link;
if (!dp_link) {
@@ -1088,14 +1096,21 @@ int dp_link_get_colorimetry_config(struct dp_link *dp_link)
link = container_of(dp_link, struct dp_link_private, dp_link);
- /*
- * Unless a video pattern CTS test is ongoing, use RGB_VESA
- * Only RGB_VESA and RGB_CEA supported for now
- */
+ /* unless a video pattern CTS test is ongoing, use CEA_VESA */
if (dp_link_is_video_pattern_requested(link))
- cc = link->dp_link.test_video.test_dyn_range;
+ dr = link->dp_link.test_video.test_dyn_range;
else
- cc = DP_TEST_DYNAMIC_RANGE_VESA;
+ dr = DP_DYNAMIC_RANGE_RGB_VESA;
+
+ /* Only RGB_VESA nd RGB_CEA supported for now */
+ switch (dr) {
+ case DP_DYNAMIC_RANGE_RGB_CEA:
+ cc = BIT(2);
+ break;
+ case DP_DYNAMIC_RANGE_RGB_VESA:
+ default:
+ cc = 0;
+ }
return cc;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 36dad05..55bb6b0 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -567,6 +567,51 @@ int dp_panel_init_panel_info(struct dp_panel *dp_panel)
return 0;
}
+/**

This marks the start of kerneldoc. But the rest of the comment isn't a kerneldoc.

+ * Mapper function which outputs colorimetry to be used for a
+ * given colorspace value when misc field of MSA is used to
+ * change the colorimetry. Currently only RGB formats have been
+ * added. This API will be extended to YUV once its supported on DP.

its != it's

+ */
+u8 dp_panel_get_misc_colorimetry_val(struct dp_panel *dp_panel)
+{
+ u8 colorimetry;
+ u32 colorspace;
+ u32 cc;
+ struct dp_panel_private *panel;
+
+ panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
+
+ cc = dp_link_get_colorimetry_config(panel->link);
+ /*
+ * If there is a non-zero value then compliance test-case
+ * is going on, otherwise we can honor the colorspace setting
+ */
+ if (cc)
+ return cc;
+
+ colorspace = dp_panel->connector->state->colorspace;
+ switch (colorspace) {
+ case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
+ case DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
+ colorimetry = 0x7;
+ break;
+ case DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED:
+ colorimetry = 0x3;
+ break;
+ case DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT:
+ colorimetry = 0xb;
+ break;
+ case DRM_MODE_COLORIMETRY_OPRGB:
+ colorimetry = 0xc;
+ break;
+ default:
+ colorimetry = 0;
+ }
+
+ return colorimetry;
+}
+
struct dp_panel *dp_panel_get(struct dp_panel_in *in)
{
struct dp_panel_private *panel;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index fb30b92..1153e88 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -85,6 +85,7 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
struct drm_connector *connector);
void dp_panel_handle_sink_request(struct dp_panel *dp_panel);
void dp_panel_tpg_config(struct dp_panel *dp_panel, bool enable);
+u8 dp_panel_get_misc_colorimetry_val(struct dp_panel *dp_panel);
/**
* is_link_rate_valid() - validates the link rate

--
With best wishes
Dmitry