[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20384cb9-8cd0-7f57-667a-27079cade833@quicinc.com>
Date: Wed, 17 Jan 2024 10:46:02 -0800
From: Kuogee Hsieh <quic_khsieh@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
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 1/17/2024 10:40 AM, Dmitry Baryshkov wrote:
> On Wed, 17 Jan 2024 at 20:29, Kuogee Hsieh <quic_khsieh@...cinc.com> wrote:
>>
>> On 1/17/2024 10:12 AM, Dmitry Baryshkov wrote:
>>> 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().
>> 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.
> Yes. But your patch doesn't fix the shift (which is correct). You
> patch fixes the value being written to that field.
ok, i will rewording the commit test
>
>>>> 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
>>>>
>
>
Powered by blists - more mailing lists