[Intel-gfx] [PATCH] drm/i915: Prune 2560x2880 mode for 5K tiled dual DP monitors

Nautiyal, Ankit K ankit.k.nautiyal at intel.com
Fri Aug 30 04:18:15 UTC 2019 


On 8/30/2019 12:06 AM, Navare, Manasi D wrote:
> On Thu, Aug 29, 2019 at 02:36:18PM +0300, Jani Nikula wrote:
>> On Thu, 29 Aug 2019, "Nautiyal, Ankit K" <ankit.k.nautiyal at intel.com> wrote:
>>> Hi Jani, Manasi,
>>>
>>> Thanks for the comments and suggestions. Please find my response inline.
>>>
>>> On 8/29/2019 12:14 PM, Jani Nikula wrote:
>>>> On Wed, 28 Aug 2019, Manasi Navare <manasi.d.navare at intel.com> wrote:
>>>>> Thanks Jani for your feedback, please see my comments inline
>>>>>
>>>>> On Wed, Aug 28, 2019 at 10:46:44AM +0300, Jani Nikula wrote:
>>>>>> On Tue, 27 Aug 2019, Manasi Navare <manasi.d.navare at intel.com> wrote:
>>>>>>> On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
>>>>>>>> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <ankit.k.nautiyal at intel.com> wrote:
>>>>>>>>> From: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>>>>>>>>>
>>>>>>>>> Currently, the transcoder port sync feature is not available, due to
>>>>>>>>> which the 5K-tiled dual DP monitors experience corruption when
>>>>>>>>> 2560x2880 mode is applied for both of the tiled DP connectors.
>>>>>>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97244
>>>>>>>>>
>>>>>>>>> There is a patch series to enable transcode port sync feature for
>>>>>>>>> tiled display for ICL+, which is under review:
>>>>>>>>> https://patchwork.kernel.org/project/intel-gfx/list/?series=137339
>>>>>>>>>
>>>>>>>>> For the older platforms, we need to remove the 2560x2880 mode to avoid
>>>>>>>>> a possibility of userspace choosing 2560x2880 mode for both tiled
>>>>>>>>> displays, resulting in corruption.
>>>>>>>>>
>>>>>>>>> This patch prunes 2560x2880 mode for one of the tiled DP connector.
>>>>>>>>> Since both the tiled DP connectors have different tile_h_loc and
>>>>>>>>> tile_v_loc, the tiled connector with tile_h_loc and tile_v_loc as '0',
>>>>>>>>> is chosen, for which the given resolution is removed.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>>>>>>>>> CC: Manasi Navare <manasi.d.navare at intel.com>
>>>>>>>>> ---
>>>>>>>>>  drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++++
>>>>>>>>>  1 file changed, 11 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>>>>>> index 5c45a3b..aa43a3b 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>>>>>> @@ -564,6 +564,17 @@ intel_dp_mode_valid(struct drm_connector *connector,
>>>>>>>>>  	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
>>>>>>>>>  		return MODE_H_ILLEGAL;
>>>>>>>>>
>>>>>>>>> +	/*
>>>>>>>>> +	 * For 5K tiled dual DP monitors, dual-DP sync is not yet supported.
>>>>>>>>> +	 * This results in display sync issues, when both tiled connectors run
>>>>>>>>> +	 * on 2560x2880 resolution. Therefore prune the 2560x2880 mode on one
>>>>>>>>> +	 * of the tiled connector, to avoid such a case.
>>>>>>>>> +	 */
>>>>>>>>> +	if (connector->has_tile &&
>>>>>>>>> +	    (connector->tile_h_loc == 0 && connector->tile_v_loc == 0) &&
>>>>>>>>> +	    (mode->hdisplay == 2560 && mode->vdisplay == 2880))
>>>>>>>>> +		return MODE_PANEL;
>>>>>>>>> +
>>>>>>>>
>>>>>>>> This assumes all tiled cases with specific resolutions fail. You don't
>>>>>>>> know that. You only know this fails on a specific display. Instead of
>>>>>>>> coming up with various rules on tiles and resolutions that match the
>>>>>>>> display (but might *also* match any number of *other* displays!), you
>>>>>>>> need to actually identify and match that specific display instead.
>>>>>>>>
>>>>>>>
>>>>>>> Actually without the transcoder port sync feature, we do not expect
>>>>>>> any tiled display over two separate ports to work correctly, so if it
>>>>>>> is two connectors in state with tile props set then we should reject
>>>>>>> the tiled mode on both those connectors since that might cause the
>>>>>>> artifacts without proper sync between two ports which is supported
>>>>>>> only on ICL+
>>>>>>
>>>>>> Consider a multi-screen display with independent panels mounted
>>>>>> together, and EDIDs set up to describe the physical tiling
>>>>>> layout. Should we reject them all because the cases you know about fail?
>>>>>>
>>>>>> You know about the issues with the specific 5k displays precisely
>>>>>> because they fail. You never hear about the ones that work. Ever. Until
>>>>>> they stop working, that is.
>>>>>
>>>>> Hmm I think even with separate panels to work without artifacts we would need some kind of
>>>>> synchronization. But yes I agree that it might just be working well and we cant assume
>>>>> that they are failing.
>>>>>
>>>>> So for now the EDID quirk sounds like the best way to fix this FDO
>>>>>
>>>>>>
>>>>>>>> There are two ways to add display specific quirks: based on EDID
>>>>>>>> (edid_quirk_list in drm_edid.c) and based on DPCD (dpcd_quirk_list in
>>>>>>>> drm_dp_helper.c). You identify the display, and then prune the modes
>>>>>>>> that require port sync to work, for *that* display.
>>>>>>>
>>>>>>> We have seen this issue on multiple 5K tiled displays IMH, so just
>>>>>>> adding a quirk for specific monitors will not suffice.
>>>>>>
>>>>>> Adding one quirk per failing display quite obviously will suffice.
>>>>>>
>>>>>>> But we would need to make sure that the mode gets rejected only if
>>>>>>> there are multiple SST connectors with tile prop or
>>>>>>> connector->has_tile set because MST tiled displays still work
>>>>>>> correctly.
>>>>>>>
>>>>>>> Ville, you had played a little bit with this 5K display I believe, do
>>>>>>> you think pruning the tiled mode if there are tiled SST connectors and
>>>>>>> platform < ICL is a good solution?
>>>>>>
>>>>>> Come to think of it, can you use the tiled mode *untiled* on one port,
>>>>>> and have it strech the entire display? There are plenty of other modes
>>>>>> you can use like this. I don't think we should reject that use case
>>>>>> either.
>>>>>
>>>>> Yes so in that case the quirk would be to set the has_tile to false so that
>>>>> the driver will actually see it as non tiled and scale it to the entire display
>>>>>
>>>>>>
>>>>>> I'll repeat, you have issues with a very specific case. You need to have
>>>>>> *very* specific rules to filter them out in order to not inadvertently
>>>>>> filter out valid use cases. Remember, if there's just *one* valid use
>>>>>> case that you end up rejecting here, it's a regression and you need to
>>>>>> revert and get back to the drawing board.
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Finally, and perhaps most importantly, there are people on the bug that
>>>>>> are going to be rather underwhelmed that after three years they get a
>>>>>> patch that simply rejects the very mode that was the reason for buying
>>>>>> the display they have. Insult to injury, the real fix is for a platform
>>>>>> that didn't exist when they bought the displays.
>>>>>
>>>>> I agree completely. Ankit could you test it on the 5K screen what happens if
>>>>> you set the has_tile to false and allow it to stretch out in untiled fashion?
>>>>> If that works we can add that to the quirk.
>>>>
>>>> I'm probably missing something here.
>>>>
>>>> Ankit lists the modes for DP-2 in [1], and among them is
>>>> 2560x2880. How's that different from using, say, 3840x2160?
>>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>> [1] http://mid.mail-archive.com/54c6c2c1-d95e-3bb4-50dd-1efff6bed7dd@intel.com
>>>>
>>>>
>>>
>>> The issue is seen only when the mode 2560x2880 is set for both of the
>>> connectors. So if we see any other combination, say 2560x2880 on DP-1,
>>> 3840x2160 on DP-2, one of the mode will cover the entire screen and
>>> there is no corruption observed. This is true for all combinations other
>>> than the (2560x2880,2560x2880) combination.
>>
>> Right, so my point was, if 2560x2880 is usable when used on one
>> connector only, it's not really proper to filter that out from the
>> modes.
>
> Hi Jani,
>
> So the has_tile or the TILE property is set per connector irrespective of the mode.
> But the way userspace handles it IMO is that if TILE prop is set and the mode matches
> the tile_h_size and tile_v_size only then it will do two atomic modesets on two connectors
> Otherwise it will just be one connector modeset.
>
> So for eg: 2560 x 2880 would be the size of each tile so for that it is doing modesets
> on two tiles and thats where we see the corruption issue for the complete total fb
> of 5120 x 2880.
> But for 3840 x 2160, that modeset just happens on a single connector and we dont see
> any issue.
>
> So i think what the quirk shd do is for this specific panel, we would not be able to
> display tiled mode together of 5120 x 2880 correctly due to corruption, so the resolution
> of 2560 x 2880 should not appear in the modelist for that connector or we just set has_tile
> to false so userspace will not try to do the tiled display magic and do a modeset on
> single connector.
>
> Ankit, could you please email me the dmesg logs in:
> 1. Case where you dont apply this patch, what happens with 2 connectors in 5K tiled mode
> 2. You force has_tile to false in the code to see the behaviour and logs in non tiled case
> 3. Prune the 2560 x 2880 mode and collect logs.
>
> I think looking at these logs will give us a clear picture and we can finalize the proper fix
>
> Regards
> Manasi
>

Sure Manasi, I'll try this and share the logs.

Regards,
Ankit

>
>>
>> BR,
>> Jani.
>>
>>
>>>
>>> I am not sure but it seems like, the monitor when receives the 2560x2880
>>> modes on both connectors, at that time the dual-dp comes to play and the
>>> corruption occurs. (I had tried to set the mode using the Ubuntu Display
>>> settings.)
>>>
>>> I had tried with Dell UP2715K monitor, I can try to get other tiled 5k
>>> monitors and check the issue without X-server on.
>>>
>>> If its Panel specific issue, its better to add quirk as suggested.
>>>
>>> Thanks & Regards,
>>> Ankit
>>>
>>>>
>>>>
>>>>>
>>>>> Manasi
>>>>>
>>>>>>
>>>>>>
>>>>>> BR,
>>>>>> Jani.
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Regards
>>>>>>> Manasi
>>>>>>>>
>>>>>>>> Blanket filters like this are a no-go.
>>>>>>>>
>>>>>>>> BR,
>>>>>>>> Jani.
>>>>>>>>
>>>>>>>>
>>>>>>>>>  	return MODE_OK;
>>>>>>>>>  }
>>>>>>>>
>>>>>>>> --
>>>>>>>> Jani Nikula, Intel Open Source Graphics Center
>>>>>>>> _______________________________________________
>>>>>>>> Intel-gfx mailing list
>>>>>>>> Intel-gfx at lists.freedesktop.org
>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>>>
>>>>>> --
>>>>>> Jani Nikula, Intel Open Source Graphics Center
>>>>
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center

More information about the Intel-gfx mailing list