[PATCH] drm/msm/dp: skip validity check for DP CTS EDID checksum

[PATCH] drm/msm/dp: skip validity check for DP CTS EDID checksum

[Jani Nikula]

The DP CTS test for EDID last block checksum expects the checksum for
the last block, invalid or not. Skip the validity check.

For the most part (*), the EDIDs returned by drm_get_edid() will be
valid anyway, and there's the CTS workaround to get the checksum for
completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
correct edid checksum after corrupted edid checksum read").

This lets us remove one user of drm_edid_block_valid() with hopes the
function can be removed altogether in the future.

(*) drm_get_edid() ignores checksum errors on CTA extensions.

Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Kuogee Hsieh <khsieh@codeaurora.org>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Stephen Boyd <swboyd@chromium.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 42d52510ffd4..86a8e06c7a60 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -289,26 +289,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
 
 static u8 dp_panel_get_edid_checksum(struct edid *edid)
 {
-	struct edid *last_block;
-	u8 *raw_edid;
-	bool is_edid_corrupt = false;
+	edid += edid->extensions;
 
-	if (!edid) {
-		DRM_ERROR("invalid edid input\n");
-		return 0;
-	}
-
-	raw_edid = (u8 *)edid;
-	raw_edid += (edid->extensions * EDID_LENGTH);
-	last_block = (struct edid *)raw_edid;
-
-	/* block type extension */
-	drm_edid_block_valid(raw_edid, 1, false, &is_edid_corrupt);
-	if (!is_edid_corrupt)
-		return last_block->checksum;
-
-	DRM_ERROR("Invalid block, no checksum\n");
-	return 0;
+	return edid->checksum;
 }
 
 void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
-- 
2.39.2

Re: [PATCH] drm/msm/dp: skip validity check for DP CTS EDID checksum

[Stephen Boyd]

Quoting Jani Nikula (2023-09-01 07:20:34)
> The DP CTS test for EDID last block checksum expects the checksum for
> the last block, invalid or not. Skip the validity check.
>
> For the most part (*), the EDIDs returned by drm_get_edid() will be
> valid anyway, and there's the CTS workaround to get the checksum for
> completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
> correct edid checksum after corrupted edid checksum read").
>
> This lets us remove one user of drm_edid_block_valid() with hopes the
> function can be removed altogether in the future.
>
> (*) drm_get_edid() ignores checksum errors on CTA extensions.
>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: Kuogee Hsieh <khsieh@codeaurora.org>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 42d52510ffd4..86a8e06c7a60 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -289,26 +289,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
>
>  static u8 dp_panel_get_edid_checksum(struct edid *edid)

It would be nice to make 'edid' const here in another patch.

Re: [PATCH] drm/msm/dp: skip validity check for DP CTS EDID checksum

[Jani Nikula]

On Thu, 07 Sep 2023, Stephen Boyd <swboyd@chromium.org> wrote:
> Quoting Jani Nikula (2023-09-01 07:20:34)
>> The DP CTS test for EDID last block checksum expects the checksum for
>> the last block, invalid or not. Skip the validity check.
>>
>> For the most part (*), the EDIDs returned by drm_get_edid() will be
>> valid anyway, and there's the CTS workaround to get the checksum for
>> completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
>> correct edid checksum after corrupted edid checksum read").
>>
>> This lets us remove one user of drm_edid_block_valid() with hopes the
>> function can be removed altogether in the future.
>>
>> (*) drm_get_edid() ignores checksum errors on CTA extensions.
>>
>> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Cc: Kuogee Hsieh <khsieh@codeaurora.org>
>> Cc: Marijn Suijten <marijn.suijten@somainline.org>
>> Cc: Rob Clark <robdclark@gmail.com>
>> Cc: Sean Paul <sean@poorly.run>
>> Cc: Stephen Boyd <swboyd@chromium.org>
>> Cc: linux-arm-msm@vger.kernel.org
>> Cc: freedreno@lists.freedesktop.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Thanks; is that enough to merge? I can't claim I would have been able to
test this.

>
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
>> index 42d52510ffd4..86a8e06c7a60 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>> @@ -289,26 +289,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
>>
>>  static u8 dp_panel_get_edid_checksum(struct edid *edid)
>
> It would be nice to make 'edid' const here in another patch.

Sure.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [Freedreno] [PATCH] drm/msm/dp: skip validity check for DP CTS EDID checksum

[Abhinav Kumar]

Hi Jani

On 9/12/2023 5:16 AM, Jani Nikula wrote:
> On Thu, 07 Sep 2023, Stephen Boyd <swboyd@chromium.org> wrote:
>> Quoting Jani Nikula (2023-09-01 07:20:34)
>>> The DP CTS test for EDID last block checksum expects the checksum for
>>> the last block, invalid or not. Skip the validity check.
>>>
>>> For the most part (*), the EDIDs returned by drm_get_edid() will be
>>> valid anyway, and there's the CTS workaround to get the checksum for
>>> completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
>>> correct edid checksum after corrupted edid checksum read").
>>>
>>> This lets us remove one user of drm_edid_block_valid() with hopes the
>>> function can be removed altogether in the future.
>>>
>>> (*) drm_get_edid() ignores checksum errors on CTA extensions.
>>>
>>> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Cc: Kuogee Hsieh <khsieh@codeaurora.org>
>>> Cc: Marijn Suijten <marijn.suijten@somainline.org>
>>> Cc: Rob Clark <robdclark@gmail.com>
>>> Cc: Sean Paul <sean@poorly.run>
>>> Cc: Stephen Boyd <swboyd@chromium.org>
>>> Cc: linux-arm-msm@vger.kernel.org
>>> Cc: freedreno@lists.freedesktop.org
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> 
> Thanks; is that enough to merge? I can't claim I would have been able to
> test this.
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

Change looks fine.

We can pick this up in the MSM tree if you would like.

Dmitry, you can please pick this up along with my R-b and Kuogee's R-b 
as well.

I think his R-b got misformatted. I can ask him to add that again.

>>
>>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
>>> index 42d52510ffd4..86a8e06c7a60 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>>> @@ -289,26 +289,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
>>>
>>>   static u8 dp_panel_get_edid_checksum(struct edid *edid)
>>
>> It would be nice to make 'edid' const here in another patch.
> 
> Sure.
> 
> BR,
> Jani.
> 
> 

Re: [Freedreno] [PATCH] drm/msm/dp: skip validity check for DP CTS EDID checksum

[Jani Nikula]

On Tue, 12 Sep 2023, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
> Hi Jani
>
> On 9/12/2023 5:16 AM, Jani Nikula wrote:
>> On Thu, 07 Sep 2023, Stephen Boyd <swboyd@chromium.org> wrote:
>>> Quoting Jani Nikula (2023-09-01 07:20:34)
>>>> The DP CTS test for EDID last block checksum expects the checksum for
>>>> the last block, invalid or not. Skip the validity check.
>>>>
>>>> For the most part (*), the EDIDs returned by drm_get_edid() will be
>>>> valid anyway, and there's the CTS workaround to get the checksum for
>>>> completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
>>>> correct edid checksum after corrupted edid checksum read").
>>>>
>>>> This lets us remove one user of drm_edid_block_valid() with hopes the
>>>> function can be removed altogether in the future.
>>>>
>>>> (*) drm_get_edid() ignores checksum errors on CTA extensions.
>>>>
>>>> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> Cc: Kuogee Hsieh <khsieh@codeaurora.org>
>>>> Cc: Marijn Suijten <marijn.suijten@somainline.org>
>>>> Cc: Rob Clark <robdclark@gmail.com>
>>>> Cc: Sean Paul <sean@poorly.run>
>>>> Cc: Stephen Boyd <swboyd@chromium.org>
>>>> Cc: linux-arm-msm@vger.kernel.org
>>>> Cc: freedreno@lists.freedesktop.org
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>> ---
>>>
>>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>> 
>> Thanks; is that enough to merge? I can't claim I would have been able to
>> test this.
>> 
>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
> Change looks fine.
>
> We can pick this up in the MSM tree if you would like.

I'd appreciate that, thanks!

BR,
Jani.

>
> Dmitry, you can please pick this up along with my R-b and Kuogee's R-b 
> as well.
>
> I think his R-b got misformatted. I can ask him to add that again.
>
>>>
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
>>>> index 42d52510ffd4..86a8e06c7a60 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>>>> @@ -289,26 +289,9 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
>>>>
>>>>   static u8 dp_panel_get_edid_checksum(struct edid *edid)
>>>
>>> It would be nice to make 'edid' const here in another patch.
>> 
>> Sure.
>> 
>> BR,
>> Jani.
>> 
>> 

-- 
Jani Nikula, Intel

Re: [PATCH] drm/msm/dp: skip validity check for DP CTS EDID checksum

[Dmitry Baryshkov]

On Fri, 01 Sep 2023 17:20:34 +0300, Jani Nikula wrote:
> The DP CTS test for EDID last block checksum expects the checksum for
> the last block, invalid or not. Skip the validity check.
> 
> For the most part (*), the EDIDs returned by drm_get_edid() will be
> valid anyway, and there's the CTS workaround to get the checksum for
> completely invalid EDIDs. See commit 7948fe12d47a ("drm/msm/dp: return
> correct edid checksum after corrupted edid checksum read").
> 
> [...]

Applied, thanks!

[1/1] drm/msm/dp: skip validity check for DP CTS EDID checksum
      https://gitlab.freedesktop.org/lumag/msm/-/commit/22e96e73182c

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>