drm/i915: Prune 2560x2880 mode for 5K tiled dual DP monitors

Commit Message

Nautiyal, Ankit K Aug. 27, 2019, 6:29 a.m. UTC

From: Ankit Nautiyal <[e-mail]>

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 <[e-mail]>
CC: Manasi Navare <[e-mail]>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Sharma, Shashank Aug. 27, 2019, 7:39 a.m. UTC | #1

Hello Ankit,

On 8/27/2019 11:59 AM, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <[e-mail]>
>
> 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 <[e-mail]>
> CC: Manasi Navare <[e-mail]>
> ---
>   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))

Shouldn't this be for >= 2560/2880 than == ? Also, do we want 
(mode->hdisplay >= 2560 *||* mode->vdisplay >= 2880 )

- Shashank

> +		return MODE_PANEL;
> +
>   	return MODE_OK;
>   }
>

Nautiyal, Ankit K Aug. 27, 2019, 9:34 a.m. UTC | #2

Hi Shashank,

Thanks for the comments.

If we see the connector info, the 5K display is detected as two DP 
connectors with the various modes (pasted below, based on some limited 
experimentation with Dell UP2715K Panel).
The problem is only observed when on both the connectors the mode 
2560x2880 is applied, looks like only then the TV monitor takes it as 5K.

Any other combination, like 3840x2160 and 2560x2880 does not have this 
problem, it seems to act something like two connectors with extended 
display configuration.

Even 2560x1440 resolution applied for both the connectors at a time, 
does not create a problem. So removing the exact-mode 2560x2880 from any 
of the connector is sufficient to avoid the corruption case.


DP-1:
modes:
"848x480": 60 29750 848 896 928 1008 480 483 488 494 0x48 0x9
"2560x2880": 60 483240 2560 2608 2640 2720 2880 2883 2893 2962 0x40 0x9
"2560x2880": 30 238240 2560 2608 2640 2720 2880 2883 2893 2921 0x40 0x9

Tile information:
has_tile = 1
tile_is_single_monitor = 1
tile_group_id = 1
num_h_tile = 2
num_v_tile = 1
tile_h_loc = 0
tile_v_loc = 0
tile_h_size = 2560
tile_v_size = 2880

DP-2:

modes:
"3840x2160": 60 533240 3840 3888 3920 4000 2160 2163 2168 2222 0x48 0x9
"2560x1440": 60 241500 2560 2608 2640 2720 1440 1443 1448 1481 0x48 0x9
"3840x2160": 60 533250 3840 3920 3952 4000 2160 2214 2219 2222 0x40 0xa
"2560x2880": 60 483240 2560 2608 2640 2720 2880 2883 2893 2962 0x40 0x9
"2560x2880": 30 238240 2560 2608 2640 2720 2880 2883 2893 2921 0x40 0x9
"1920x1200": 60 193250 1920 2056 2256 2592 1200 1203 1209 1245 0x40 0x6
"1920x1080": 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x40 0xa
"1600x1200": 60 162000 1600 1664 1856 2160 1200 1201 1204 1250 0x40 0x5
"1680x1050": 60 146250 1680 1784 1960 2240 1050 1053 1059 1089 0x40 0x6
"1280x1024": 60 108000 1280 1328 1440 1688 1024 1025 1028 1066 0x40 0x5
"1280x800": 60 83500 1280 1352 1480 1680 800 803 809 831 0x40 0x6
"1024x768": 60 65000 1024 1048 1184 1344 768 771 777 806 0x40 0xa
"800x600": 60 40000 800 840 968 1056 600 601 605 628 0x40 0x5
"640x480": 60 25200 640 656 752 800 480 490 492 525 0x40 0xa
"640x480": 60 25175 640 656 752 800 480 490 492 525 0x40 0xa

Tile information:
has_tile = 1
tile_is_single_monitor = 1
tile_group_id = 1
num_h_tile = 2
num_v_tile = 1
tile_h_loc = 1
tile_v_loc = 0
tile_h_size = 2560
tile_v_size = 2880

Regards,
Ankit

On 8/27/2019 1:09 PM, Sharma, Shashank wrote:
> Hello Ankit,
>
> On 8/27/2019 11:59 AM, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <[e-mail]>
>>
>> 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 <[e-mail]>
>> CC: Manasi Navare <[e-mail]>
>> ---
>>  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))
>
> Shouldn't this be for >= 2560/2880 than == ? Also, do we want
> (mode->hdisplay >= 2560 *||* mode->vdisplay >= 2880 )
>
> - Shashank
>
>> +		return MODE_PANEL;
>> +
>>  	return MODE_OK;
>>  }
>>

Jani Nikula Aug. 27, 2019, 10:34 a.m. UTC | #3

On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
> From: Ankit Nautiyal <[e-mail]>
>
> 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 <[e-mail]>
> CC: Manasi Navare <[e-mail]>
> ---
>  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.

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.

Blanket filters like this are a no-go.

BR,
Jani.


>  	return MODE_OK;
>  }

Navare, Manasi Aug. 27, 2019, 4:59 p.m. UTC | #4

On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
> > From: Ankit Nautiyal <[e-mail]>
> >
> > 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 <[e-mail]>
> > CC: Manasi Navare <[e-mail]>
> > ---
> >  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+
 
> 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.

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?

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
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula Aug. 28, 2019, 7:46 a.m. UTC | #5

On Tue, 27 Aug 2019, Manasi Navare <[e-mail]> wrote:
> On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
>> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
>> > From: Ankit Nautiyal <[e-mail]>
>> >
>> > 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 <[e-mail]>
>> > CC: Manasi Navare <[e-mail]>
>> > ---
>> >  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.

>> 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.

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.


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
>> [e-mail]
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Navare, Manasi Aug. 28, 2019, 7:53 p.m. UTC | #6

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 <[e-mail]> wrote:
> > On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
> >> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
> >> > From: Ankit Nautiyal <[e-mail]>
> >> >
> >> > 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 <[e-mail]>
> >> > CC: Manasi Navare <[e-mail]>
> >> > ---
> >> >  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.

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
> >> [e-mail]
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Jani Nikula Aug. 29, 2019, 6:44 a.m. UTC | #7

On Wed, 28 Aug 2019, Manasi Navare <[e-mail]> 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 <[e-mail]> wrote:
>> > On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
>> >> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
>> >> > From: Ankit Nautiyal <[e-mail]>
>> >> >
>> >> > 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 <[e-mail]>
>> >> > CC: Manasi Navare <[e-mail]>
>> >> > ---
>> >> >  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/[e-mail]




>
> 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
>> >> [e-mail]
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

Nautiyal, Ankit K Aug. 29, 2019, 9:09 a.m. UTC | #8

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 <[e-mail]> 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 <[e-mail]> wrote:
>>>> On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
>>>>> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
>>>>>> From: Ankit Nautiyal <[e-mail]>
>>>>>>
>>>>>> 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 <[e-mail]>
>>>>>> CC: Manasi Navare <[e-mail]>
>>>>>> ---
>>>>>>  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/[e-mail]
>
>

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.

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
>>>>> [e-mail]
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>> --
>>> Jani Nikula, Intel Open Source Graphics Center
>

Jani Nikula Aug. 29, 2019, 11:36 a.m. UTC | #9

On Thu, 29 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> 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 <[e-mail]> 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 <[e-mail]> wrote:
>>>>> On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
>>>>>> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
>>>>>>> From: Ankit Nautiyal <[e-mail]>
>>>>>>>
>>>>>>> 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 <[e-mail]>
>>>>>>> CC: Manasi Navare <[e-mail]>
>>>>>>> ---
>>>>>>>  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/[e-mail]
>>
>>
>
> 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.

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
>>>>>> [e-mail]
>>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>
>>>> --
>>>> Jani Nikula, Intel Open Source Graphics Center
>>

Navare, Manasi Aug. 29, 2019, 6:36 p.m. UTC | #10

On Thu, Aug 29, 2019 at 02:36:18PM +0300, Jani Nikula wrote:
> On Thu, 29 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> 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 <[e-mail]> 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 <[e-mail]> wrote:
> >>>>> On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
> >>>>>> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
> >>>>>>> From: Ankit Nautiyal <[e-mail]>
> >>>>>>>
> >>>>>>> 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 <[e-mail]>
> >>>>>>> CC: Manasi Navare <[e-mail]>
> >>>>>>> ---
> >>>>>>>  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/[e-mail]
> >>
> >>
> >
> > 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


> 
> 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
> >>>>>> [e-mail]
> >>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>>>
> >>>> --
> >>>> Jani Nikula, Intel Open Source Graphics Center
> >>
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Nautiyal, Ankit K Aug. 30, 2019, 4:18 a.m. UTC | #11

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" <[e-mail]> 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 <[e-mail]> 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 <[e-mail]> wrote:
>>>>>>> On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
>>>>>>>> On Tue, 27 Aug 2019, "Nautiyal, Ankit K" <[e-mail]> wrote:
>>>>>>>>> From: Ankit Nautiyal <[e-mail]>
>>>>>>>>>
>>>>>>>>> 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 <[e-mail]>
>>>>>>>>> CC: Manasi Navare <[e-mail]>
>>>>>>>>> ---
>>>>>>>>>  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/[e-mail]
>>>>
>>>>
>>>
>>> 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
>>>>>>>> [e-mail]
>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>>>
>>>>>> --
>>>>>> Jani Nikula, Intel Open Source Graphics Center
>>>>
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center

Nautiyal, Ankit K Sept. 5, 2019, 5:33 a.m. UTC | #12

Hi,

I was able to get 5K HPz27q 317b monitor for some time. Below are the 
observation on HPz27q Monitor with two DP cables connected to a KBL machine.

*****General Observation*****
The monitor settings has two modes, DP1.0 and DP1.2.
One of the connector is enumerated as 'tiled' and the other as non tiled.

The non-tiled connector has modes starting from 2K and below, and the 
tiled connector has just one mode 2560x2880.
No corruption observed in this case.

In case of DP2.0 two connectors are enumerated, both tiled.
One connector has modes from 3849x2160 and below. 2560x2880 being 
preferred mode.
The other has 2560x2880 mode, also preferred.

The issue is seen when both the modes selected are 2560x2880. This 
results like two halves of screens not in sync.

*****Experiment with different patches*****

As discussed I collected logs in 3 cases:
1. Without any patch (vanilla)
2. With patch to prune the 2560x2880 mode, only for tile with HLOC and 
VLOC as 0.
3. With a patch to force the connector property as 'false'

Logs for which are attached in fdo bug #97244 
https://bugs.freedesktop.org/show_bug.cgi?id=97244
https://bugs.freedesktop.org/attachment.cgi?id=145267

Note 1: I had changed the display info to provide the Tile information, 
in case the connector 'has_tile' is true.
Note 2: I had checked and collected logs with single display and also 
with dual display configuration with DP1.2 monitor settings.
Note 3: The mode is changed using xrandr.

case1:
-Without any patch : 2560x2880 modeset on both connectors causes corruption.

case2:
-With 2560x2880 mode pruned for one of the tile : Only one of the 
connector shows 2560x2880 mode.
2560x2880 modeset on any the remaining connector resulted in blank screen.
Any other modeset works.

case3:
-With has_tile connector property forcibly reset : The connector are 
listed as not tiled but still, 2560x2880 modeset on any the connectors 
causes blank screen.
Any other modeset works.

To summarize, pruning on just one tiled connector does not solve the 
issue, if we need to prune, we need to do it for both the connectors.
Secondly, the forcible setting of has_tile = 'false' also, does not 
help, and resulted in blank screen when 2560x2880 mode is applied.
So IMHO if we need to prune the mode 2560x2880, we need to prune it for 
both the connectors.

Regards,
Ankit


On 8/30/2019 9:48 AM, Nautiyal, Ankit K wrote:
>
>
> 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" <[e-mail]>
>>> 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 <[e-mail]> 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 <[e-mail]>
>>>>>>> wrote:
>>>>>>>> On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
>>>>>>>>> On Tue, 27 Aug 2019, "Nautiyal, Ankit K"
>>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>> From: Ankit Nautiyal <[e-mail]>
>>>>>>>>>>
>>>>>>>>>> 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 <[e-mail]>
>>>>>>>>>> CC: Manasi Navare <[e-mail]>
>>>>>>>>>> ---
>>>>>>>>>>  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/[e-mail]
>>>>>
>>>>>
>>>>>
>>>>
>>>> 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
>>>>>>>>> [e-mail]
>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>>>>
>>>>>>> --
>>>>>>> Jani Nikula, Intel Open Source Graphics Center
>>>>>
>>>
>>> --
>>> Jani Nikula, Intel Open Source Graphics Center

Navare, Manasi Sept. 9, 2019, 6:42 p.m. UTC | #13

On Thu, Sep 05, 2019 at 11:03:12AM +0530, Nautiyal, Ankit K wrote:
> Hi,
> 
> I was able to get 5K HPz27q 317b monitor for some time. Below are the
> observation on HPz27q Monitor with two DP cables connected to a KBL machine.
> 
> *****General Observation*****
> The monitor settings has two modes, DP1.0 and DP1.2.
> One of the connector is enumerated as 'tiled' and the other as non tiled.
> 
> The non-tiled connector has modes starting from 2K and below, and the tiled
> connector has just one mode 2560x2880.
> No corruption observed in this case.
> 
> In case of DP2.0 two connectors are enumerated, both tiled.
> One connector has modes from 3849x2160 and below. 2560x2880 being preferred
> mode.
> The other has 2560x2880 mode, also preferred.
> 
> The issue is seen when both the modes selected are 2560x2880. This results
> like two halves of screens not in sync.
> 
> *****Experiment with different patches*****
> 
> As discussed I collected logs in 3 cases:
> 1. Without any patch (vanilla)
> 2. With patch to prune the 2560x2880 mode, only for tile with HLOC and VLOC
> as 0.
> 3. With a patch to force the connector property as 'false'
> 
> Logs for which are attached in fdo bug #97244
> https://bugs.freedesktop.org/show_bug.cgi?id=97244
> https://bugs.freedesktop.org/attachment.cgi?id=145267
> 
> Note 1: I had changed the display info to provide the Tile information, in
> case the connector 'has_tile' is true.
> Note 2: I had checked and collected logs with single display and also with
> dual display configuration with DP1.2 monitor settings.
> Note 3: The mode is changed using xrandr.
> 
> case1:
> -Without any patch : 2560x2880 modeset on both connectors causes corruption.
> 
> case2:
> -With 2560x2880 mode pruned for one of the tile : Only one of the connector
> shows 2560x2880 mode.
> 2560x2880 modeset on any the remaining connector resulted in blank screen.
> Any other modeset works.
> 
> case3:
> -With has_tile connector property forcibly reset : The connector are listed
> as not tiled but still, 2560x2880 modeset on any the connectors causes blank
> screen.
> Any other modeset works.
> 
> To summarize, pruning on just one tiled connector does not solve the issue,
> if we need to prune, we need to do it for both the connectors.
> Secondly, the forcible setting of has_tile = 'false' also, does not help,
> and resulted in blank screen when 2560x2880 mode is applied.
> So IMHO if we need to prune the mode 2560x2880, we need to prune it for both
> the connectors.
>

Thanks Ankit for these experiments and logs.
So looks like the solution would be to add a quirk for thsi specific EDID for
the panel and then  prune 2560x2880 on both connectors.

Jani does that sound like a good acceptable solution?

Manasi
 
> Regards,
> Ankit
> 
> 
> On 8/30/2019 9:48 AM, Nautiyal, Ankit K wrote:
> >
> >
> >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" <[e-mail]>
> >>>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 <[e-mail]> 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 <[e-mail]>
> >>>>>>>wrote:
> >>>>>>>>On Tue, Aug 27, 2019 at 01:34:15PM +0300, Jani Nikula wrote:
> >>>>>>>>>On Tue, 27 Aug 2019, "Nautiyal, Ankit K"
> >>>>>>>>><[e-mail]> wrote:
> >>>>>>>>>>From: Ankit Nautiyal <[e-mail]>
> >>>>>>>>>>
> >>>>>>>>>>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 <[e-mail]>
> >>>>>>>>>>CC: Manasi Navare <[e-mail]>
> >>>>>>>>>>---
> >>>>>>>>>> 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/[e-mail]
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>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
> >>>>>>>>>[e-mail]
> >>>>>>>>>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>>>>>>
> >>>>>>>--
> >>>>>>>Jani Nikula, Intel Open Source Graphics Center
> >>>>>
> >>>
> >>>--
> >>>Jani Nikula, Intel Open Source Graphics Center

Jani Nikula Sept. 10, 2019, 9:20 a.m. UTC | #14

On Mon, 09 Sep 2019, Manasi Navare <[e-mail]> wrote:
> On Thu, Sep 05, 2019 at 11:03:12AM +0530, Nautiyal, Ankit K wrote:
>> Hi,
>> 
>> I was able to get 5K HPz27q 317b monitor for some time. Below are the
>> observation on HPz27q Monitor with two DP cables connected to a KBL machine.
>> 
>> *****General Observation*****
>> The monitor settings has two modes, DP1.0 and DP1.2.
>> One of the connector is enumerated as 'tiled' and the other as non tiled.
>> 
>> The non-tiled connector has modes starting from 2K and below, and the tiled
>> connector has just one mode 2560x2880.
>> No corruption observed in this case.
>> 
>> In case of DP2.0 two connectors are enumerated, both tiled.
>> One connector has modes from 3849x2160 and below. 2560x2880 being preferred
>> mode.
>> The other has 2560x2880 mode, also preferred.
>> 
>> The issue is seen when both the modes selected are 2560x2880. This results
>> like two halves of screens not in sync.
>> 
>> *****Experiment with different patches*****
>> 
>> As discussed I collected logs in 3 cases:
>> 1. Without any patch (vanilla)
>> 2. With patch to prune the 2560x2880 mode, only for tile with HLOC and VLOC
>> as 0.
>> 3. With a patch to force the connector property as 'false'
>> 
>> Logs for which are attached in fdo bug #97244
>> https://bugs.freedesktop.org/show_bug.cgi?id=97244
>> https://bugs.freedesktop.org/attachment.cgi?id=145267
>> 
>> Note 1: I had changed the display info to provide the Tile information, in
>> case the connector 'has_tile' is true.
>> Note 2: I had checked and collected logs with single display and also with
>> dual display configuration with DP1.2 monitor settings.
>> Note 3: The mode is changed using xrandr.
>> 
>> case1:
>> -Without any patch : 2560x2880 modeset on both connectors causes corruption.
>> 
>> case2:
>> -With 2560x2880 mode pruned for one of the tile : Only one of the connector
>> shows 2560x2880 mode.
>> 2560x2880 modeset on any the remaining connector resulted in blank screen.
>> Any other modeset works.
>> 
>> case3:
>> -With has_tile connector property forcibly reset : The connector are listed
>> as not tiled but still, 2560x2880 modeset on any the connectors causes blank
>> screen.
>> Any other modeset works.
>> 
>> To summarize, pruning on just one tiled connector does not solve the issue,
>> if we need to prune, we need to do it for both the connectors.
>> Secondly, the forcible setting of has_tile = 'false' also, does not help,
>> and resulted in blank screen when 2560x2880 mode is applied.
>> So IMHO if we need to prune the mode 2560x2880, we need to prune it for both
>> the connectors.
>>
>
> Thanks Ankit for these experiments and logs.
> So looks like the solution would be to add a quirk for thsi specific EDID for
> the panel and then  prune 2560x2880 on both connectors.
>
> Jani does that sound like a good acceptable solution?

So you'd have to disable the mode for both displays, but then it would
seem odd to expose the tiling configuration with a non-existing mode,
wouldn't it?

The quirk should be: on these displays, disable tiling and prune the
modes listed in the tile info. That's a fairly generic quirk that makes
sense to *define* in drm. However you'd only *implement* that quirk in
affected drivers, i.e. i915.

The question remains, what does it take to actually fix this? I haven't
looked at the patches in detail, what makes them so icl specific?

I'll repeat what I said up-thread:

On Wed, 28 Aug 2019, Jani Nikula <[e-mail]> wrote:
> 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.

Some of those folks apparently can actually make it work when they try
hard enough. You'd take that away too.


BR,
Jani.

Navare, Manasi Sept. 10, 2019, 9:32 p.m. UTC | #15

On Tue, Sep 10, 2019 at 12:20:05PM +0300, Jani Nikula wrote:
> On Mon, 09 Sep 2019, Manasi Navare <[e-mail]> wrote:
> > On Thu, Sep 05, 2019 at 11:03:12AM +0530, Nautiyal, Ankit K wrote:
> >> Hi,
> >> 
> >> I was able to get 5K HPz27q 317b monitor for some time. Below are the
> >> observation on HPz27q Monitor with two DP cables connected to a KBL machine.
> >> 
> >> *****General Observation*****
> >> The monitor settings has two modes, DP1.0 and DP1.2.
> >> One of the connector is enumerated as 'tiled' and the other as non tiled.
> >> 
> >> The non-tiled connector has modes starting from 2K and below, and the tiled
> >> connector has just one mode 2560x2880.
> >> No corruption observed in this case.
> >> 
> >> In case of DP2.0 two connectors are enumerated, both tiled.
> >> One connector has modes from 3849x2160 and below. 2560x2880 being preferred
> >> mode.
> >> The other has 2560x2880 mode, also preferred.
> >> 
> >> The issue is seen when both the modes selected are 2560x2880. This results
> >> like two halves of screens not in sync.
> >> 
> >> *****Experiment with different patches*****
> >> 
> >> As discussed I collected logs in 3 cases:
> >> 1. Without any patch (vanilla)
> >> 2. With patch to prune the 2560x2880 mode, only for tile with HLOC and VLOC
> >> as 0.
> >> 3. With a patch to force the connector property as 'false'
> >> 
> >> Logs for which are attached in fdo bug #97244
> >> https://bugs.freedesktop.org/show_bug.cgi?id=97244
> >> https://bugs.freedesktop.org/attachment.cgi?id=145267
> >> 
> >> Note 1: I had changed the display info to provide the Tile information, in
> >> case the connector 'has_tile' is true.
> >> Note 2: I had checked and collected logs with single display and also with
> >> dual display configuration with DP1.2 monitor settings.
> >> Note 3: The mode is changed using xrandr.
> >> 
> >> case1:
> >> -Without any patch : 2560x2880 modeset on both connectors causes corruption.
> >> 
> >> case2:
> >> -With 2560x2880 mode pruned for one of the tile : Only one of the connector
> >> shows 2560x2880 mode.
> >> 2560x2880 modeset on any the remaining connector resulted in blank screen.
> >> Any other modeset works.
> >> 
> >> case3:
> >> -With has_tile connector property forcibly reset : The connector are listed
> >> as not tiled but still, 2560x2880 modeset on any the connectors causes blank
> >> screen.
> >> Any other modeset works.
> >> 
> >> To summarize, pruning on just one tiled connector does not solve the issue,
> >> if we need to prune, we need to do it for both the connectors.
> >> Secondly, the forcible setting of has_tile = 'false' also, does not help,
> >> and resulted in blank screen when 2560x2880 mode is applied.
> >> So IMHO if we need to prune the mode 2560x2880, we need to prune it for both
> >> the connectors.
> >>
> >
> > Thanks Ankit for these experiments and logs.
> > So looks like the solution would be to add a quirk for thsi specific EDID for
> > the panel and then  prune 2560x2880 on both connectors.
> >
> > Jani does that sound like a good acceptable solution?
> 
> So you'd have to disable the mode for both displays, but then it would
> seem odd to expose the tiling configuration with a non-existing mode,
> wouldn't it?
> 
> The quirk should be: on these displays, disable tiling and prune the
> modes listed in the tile info. That's a fairly generic quirk that makes
> sense to *define* in drm. However you'd only *implement* that quirk in
> affected drivers, i.e. i915.
> 
> The question remains, what does it take to actually fix this? I haven't
> looked at the patches in detail, what makes them so icl specific?
>

This made me go back and reconnect with the HW teams and get some clarifications.
Looks like the transcoder port sync feature for DP SST was added since BDW
and only the MST support starts from ICL+, but this was not clearly called out
in the bspec and hence the confusion.

I will update my transcoder port sync patches : https://patchwork.freedesktop.org/series/66403/
to add this support starting BDW
and Ankit you could then test it on the 5K monitor and it should fix it.

Manasi
 
> I'll repeat what I said up-thread:
> 
> On Wed, 28 Aug 2019, Jani Nikula <[e-mail]> wrote:
> > 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.
> 
> Some of those folks apparently can actually make it work when they try
> hard enough. You'd take that away too.
> 
> 
> BR,
> Jani.
> 
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Patch

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;
+
 	return MODE_OK;
 }