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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA8EJpqO+SWYcCtH1hOVow18ZTbJ=qh0D68CEP1sGObdd3=ciA@mail.gmail.com>
Date: Wed, 17 Jan 2024 20:12:28 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Kuogee Hsieh <quic_khsieh@...cinc.com>
Cc: dri-devel@...ts.freedesktop.org, robdclark@...il.com, sean@...rly.run, 
	swboyd@...omium.org, dianders@...omium.org, vkoul@...nel.org, daniel@...ll.ch, 
	airlied@...il.com, agross@...nel.org, andersson@...nel.org, 
	quic_abhinavk@...cinc.com, quic_jesszhan@...cinc.com, 
	quic_sbillaka@...cinc.com, marijn.suijten@...ainline.org, 
	freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm/dp: correct configure Colorimetry Indicator
 Field at MISC0

On Wed, 17 Jan 2024 at 19:54, Kuogee Hsieh <quic_khsieh@...cinc.com> 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().

> 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@...cinc.com>
> ---
>  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
>


-- 
With best wishes
Dmitry