[SRU][OEM-5.14/Jammy/OEM-5.17][PATCH 0/1] Fix blank screen on Thinkpad ADL 4K+ panel

[Stefan Bader]

On 12.07.22 14:44, Aaron Ma wrote:
> 
> 
> On 7/8/22 17:12, Stefan Bader wrote:
>> On 04.07.22 07:15, Aaron Ma wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1980621
>>>
>>> [Impact]
>>> Set screen off on ThinkPad P1G5 with 4k+ panel.
>>> The screen will never be back.
>>>
>>> [Fix]
>>> It's a upstream kernel regression.
>>> And reverted by upstream.
>>>
>>> [Test]
>>> Verified on hardware, screen on/off 20 times OK.
>>>
>>> [Where problems could occur]
>>> Low risk, it reverts a regression and may break i915 driver.
>>>
>>> On 5.14-oem and Jammy kernel, This commit was claimed to backport
>>> commit 73867c8709b5 ("drm/i915/display: Remove check for low voltage sku for 
>>> max dp source rate")
>>> But it's not kind of it.
>>> So revert it instead.
>>>
>>> On 5.17 kernel, revert it by upstream commit.
>>>
>>> Unstable kernel already got it.
>>>
>>> Jason A. Donenfeld (1):
>>>    drm/i915/display: Re-add check for low voltage sku for max dp source
>>>      rate
>>>
>>> Aaron Ma (1):
>>>    UBUNTU: SAUCE: Revert "drm/i915/display: Remove check for low voltage
>>>      sku for max dp source rate"
>>>
>>>   drivers/gpu/drm/i915/display/intel_dp.c | 32 ++++++++++++++++++++++---
>>>   1 file changed, 29 insertions(+), 3 deletions(-)
>>>
>>
>> This would be a case where it would be good also say what was the original 
>> problem that the patch to be reverted tried to solve. But also the fact that 
>> the patch in Jammy claims to be a cherry pick but is completely different from 
>> the upstream patch, yet the SHA1 _are_ identical.
>>
> 
> The original problem did fix a problem but introduced a new one.
> Intel already sent another new fix the original issue:
> https://patchwork.freedesktop.org/series/105573/
> 
>> So how can we trust any HWE submission that says cherry pick in the future?
>> What does the revert in Jammy _actually_ do? Both patches are completely 
>> different, so is Jammy really affected by the issues which the upstream patch 
>> caused?
> 
> The revert is trying to be back to  540000 instead of 810000 on all ADL+ platform.

This ^ is total gibberish if one does not spend a lot of time inside hardware 
specs. Yes, it is some "rate" but what this has to do with low voltage SKUs and 
all the other random words I do not know and I won't have time to explore when 
doing patch reviews.

> 
> The cherry-pick patch did the same thing as the commit ID.
> It looks like a backport and but no mention.
>

It might end up doing the same thing but there is no way I could tell just from 
looking at the code. That is why it is so vitally important to do the patch 
annotations properly. And not only correctly state something is a backport but 
also explain why it is different.

>>
>> The Jammy patch probably should be reverted under that commit message. The 
>> question is, should the code get re-applied again with the _CORRECT_ provenance?
> 
> No need to re-applied again this patch, new patch above is coming as a real fix.

I will take your word here that dropping that additional "rate" setting ends up 
using a lower "rate" and that is the way things were before and work better. And 
that the effect is only that for now one cannot set 4K on dual screens which is 
a lesser evil than not being able to suspend and come back.

-Stefan

> 
> Regards,
> Aaron
> 
>>
>> -Stefan
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20220713/ffd76a03/attachment.sig>