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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> wrote:
From: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>

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@xxxxxxxxx>
CC: Manasi Navare <manasi.d.navare@xxxxxxxxx>
---
 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@xxxxxxxxx



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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Jani Nikula, Intel Open Source Graphics Center


--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux