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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/17/2024 10:12 AM, Dmitry Baryshkov wrote:
On Wed, 17 Jan 2024 at 19:54, Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> wrote:
MSA MISC0 bit 1 to 7 contains Colorimetry Indicator Field. At
current implementation, at DP_TEST_DYNAMIC_RANGE_CEA case the
In the current implementation, in the ... case

Colorimetry Indicator Field is mistakenly left shifted one extra
bit.
This doesn't make sense. You say that the value is mistakenly shifted,
but the shift is still in place in dp_catalog_ctrl_config_misc().

The problem is at

 link->dp_link.test_video.test_dyn_range =   (bp & DP_TEST_DYNAMIC_RANGE_CEA);   <== this from reading dpcd directly where ==> DP_TEST_DYNAMIC_RANGE_CEA  is   (1 << 3)

within dp_catalog_ctrl_config_misc(), cc will be left shift one more bit.
so that cc is totally is left shifted 4 bits (bit 4).

at misc0, it should be bit 3 set only for CEA_RGB.


This patch return correct value of colorimetry at
dp_link_get_colorimetry_config() to fix this problem.
See Documentation/process/submitting-patches.rst#_describe_changes

Changes in V2:
-- drop retrieving colorimetry from colorspace
-- drop dr = link->dp_link.test_video.test_dyn_range assignment

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
  drivers/gpu/drm/msm/dp/dp_link.c | 11 ++++++-----
  drivers/gpu/drm/msm/dp/dp_link.h |  3 +++
  2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 98427d4..2e1bdaf 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -1082,7 +1082,7 @@ int dp_link_process_request(struct dp_link *dp_link)

  int dp_link_get_colorimetry_config(struct dp_link *dp_link)
  {
-       u32 cc;
+       u32 cc = DP_MISC0_LEGACY_RGB;
         struct dp_link_private *link;

         if (!dp_link) {
@@ -1096,10 +1096,11 @@ int dp_link_get_colorimetry_config(struct dp_link *dp_link)
          * Unless a video pattern CTS test is ongoing, use RGB_VESA
          * Only RGB_VESA and RGB_CEA supported for now
          */
-       if (dp_link_is_video_pattern_requested(link))
-               cc = link->dp_link.test_video.test_dyn_range;
-       else
-               cc = DP_TEST_DYNAMIC_RANGE_VESA;
+       if (dp_link_is_video_pattern_requested(link)) {
+               if (link->dp_link.test_video.test_dyn_range &
+                                       DP_TEST_DYNAMIC_RANGE_CEA)
+                       cc = DP_MISC0_CEA_RGB;
+       }

         return cc;
  }
diff --git a/drivers/gpu/drm/msm/dp/dp_link.h b/drivers/gpu/drm/msm/dp/dp_link.h
index 9dd4dd9..fe8f716 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.h
+++ b/drivers/gpu/drm/msm/dp/dp_link.h
@@ -12,6 +12,9 @@
  #define DP_TEST_BIT_DEPTH_UNKNOWN 0xFFFFFFFF
  #define DP_LINK_CAP_ENHANCED_FRAMING (1 << 0)

+#define DP_MISC0_LEGACY_RGB            0
+#define DP_MISC0_CEA_RGB               0x04
These should go to dp_reg.h and should start with DP_MISC0_COLORIMETRY_CFG

+
  struct dp_link_info {
         unsigned char revision;
         unsigned int rate;
--
2.7.4





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux