NACK/Cmnt: [PATCH 00/18][SRU][G][HWE-5.8] Fix compatibility issue on some external monitors

[Aaron Ma]

On 4/8/21 2:07 PM, Stefan Bader wrote:
> On 08.04.21 07:56, Stefan Bader wrote:
>> On 23.03.21 09:27, Stefan Bader wrote:
>>> On 22.03.21 16:47, Aaron Ma wrote:
>>>> BugLink: https://bugs.launchpad.net/bugs/1920777
>>>>
>>>> [Impact]
>>>> Some external monitors got black screen via HDMI on laptops with Intel
>>>> GPU.
>>>>
>>>> [Fix]
>>>> Reading more of the port caps and dealing with various clock/bpc
>>>> limitations.
>>>> Patchset:
>>>> https://patchwork.freedesktop.org/series/72928/
>>>>
>>>> [Test Case]
>>>> Verified on the target platform and other 2 laptops include workstation,
>>>> Tested on LG/ThinkVision 4k monitors and Dell 1080p monitor, include
>>>> dock station.
>>>> All good.
>>>>
>>>> [Where problems could occur]
>>>> Intel GPU may lose or mess the output.
>>>>
>>>> These patches are most backported from 5.10-rc1 and one from 5.11-rc5.
>>>> Hirsute/Unstable already got them.
>>>>
>>>> Ville Syrjälä (18):
>>>>    drm/i915/lspcon: Do not send infoframes to non-HDMI sinks
>>>>    drm/dp: Define protocol converter DPCD registers
>>>>    drm/dp: Define more downstream facing port caps
>>>>    drm/i915: Reworkd DFP max bpc handling
>>>>    drm/dp: Add helpers to identify downstream facing port types
>>>>    drm/dp: Pimp drm_dp_downstream_max_bpc()
>>>>    drm/dp: Redo drm_dp_downstream_max_clock() as
>>>>      drm_dp_downstream_max_dotclock()
>>>>    drm/i915: Reworkd DP DFP clock handling
>>>>    drm/dp: Add drm_dp_downstream_{min,max}_tmds_clock()
>>>>    drm/i915: Deal with TMDS DFP clock limits
>>>>    drm/i915: Configure DP 1.3+ protocol converted HDMI mode
>>>>    drm/dp: Add drm_dp_downstream_mode()
>>>>    drm/i915: Handle downstream facing ports w/o EDID
>>>>    drm/i915: Extract intel_hdmi_has_audio()
>>>>    drm/i915: DP->HDMI TMDS clock limits vs. deep color
>>>>    drm/dp: Add helpers for DFP YCbCr 4:2:0 handling
>>>>    drm/i915: Do YCbCr 444->420 conversion via DP protocol converters
>>>>    drm/i915: Only enable DFP 4:4:4->4:2:0 conversion when outputting
>>>>      YCbCr 4:4:4
>>>>
>>>>   drivers/gpu/drm/drm_dp_helper.c               | 381 ++++++++++++++++--
>>>>   drivers/gpu/drm/drm_edid.c                    |  28 ++
>>>>   drivers/gpu/drm/i915/display/intel_ddi.c      |  11 +-
>>>>   .../drm/i915/display/intel_display_debugfs.c  |   3 +-
>>>>   .../drm/i915/display/intel_display_types.h    |   9 +
>>>>   drivers/gpu/drm/i915/display/intel_dp.c       | 305 +++++++++++---
>>>>   drivers/gpu/drm/i915/display/intel_dp.h       |   2 +
>>>>   drivers/gpu/drm/i915/display/intel_hdmi.c     |  82 ++--
>>>>   drivers/gpu/drm/i915/display/intel_hdmi.h     |   2 +
>>>>   include/drm/drm_dp_helper.h                   |  63 ++-
>>>>   include/drm/drm_edid.h                        |   4 +
>>>>   11 files changed, 758 insertions(+), 132 deletions(-)
>>>>
>>> First why is this explicitly targeted to focal:linux-hwe-5.8 when also asked to be applied to groovy:linux? Hwe kernels are and always have been backports from the respective series primary kernel.
>>> Second, is this really the minimal set required to fix the problems? This looks more to be a partial rework and for that testing only on 3 machines (which does not say anything about which generations of Intel graphics get covered). For a change that huge I would rather expect to cover the whole range of generations we still have around.
>>> The patches seem to origin from 5.10..5.11 which means the broadest range of testing would not happen before Hirsute does release. Before that I am rather suspicious the bigger the change set becomes.
>>>
>>> -Stefan
>>>
>>>
>> There has not been any update on this and this still is a rather extensive set. Bug fixes should try to be as small as possible because the bigger the change the greater the risk to cause some unforeseen problems.
>> In some cases this maybe is not possible. But then this should be explained in more detail and also testing should be at a wider range. Especially with i915 which has been breaking older generations of the GPU.
>> In this case it seems the better option is to wait for the hirsute/5.11 kernel to replace the groovy/5.8 one as focal hwe kernel.
>>

Agree, newer kernel should be a better option.

Thanks for your review.
Aaron

>> -Stefan
>>
>>
> PS: Generally, if a set really needs to consist of more than ~5 patches, it would be preferable to have a pull request at least embedded in the cover email (or just a pull request style submission overall).
> 
>