[PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

[PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

[Anusha Srivatsa]

Change intel_dp_mst_mode_valid() to use available link bandwidth
rather than the link's maximum supported bandwidth to evaluate
whether modes are legal for the current configuration. This takes
into account the fact that link bandwidth may already be dedicated
to other virtual channels.

Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 629337d..39c58eb 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -352,16 +352,22 @@ static enum drm_mode_status
 intel_dp_mst_mode_valid(struct drm_connector *connector,
 			struct drm_display_mode *mode)
 {
-	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
+	int req_pbn = 0;
+	int slots = 0;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_dp *intel_dp = intel_connector->mst_port;
+	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
+
+	req_pbn = drm_dp_calc_pbn_mode(mode->clock, 24);
+	slots = drm_dp_find_vcpi_slots(mgr, req_pbn);
 
-	/* TODO - validate mode against available PBN for link */
 	if (mode->clock < 10000)
 		return MODE_CLOCK_LOW;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		return MODE_H_ILLEGAL;
 
-	if (mode->clock > max_dotclk)
+	if (slots == -ENOSPC)
 		return MODE_CLOCK_HIGH;
 
 	return MODE_OK;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for drm/i915/dp/mst: Validate modes against the available link bandwidth

[Patchwork]

== Series Details ==

Series: drm/i915/dp/mst: Validate modes against the available link bandwidth
URL   : https://patchwork.freedesktop.org/series/11298/
State : failure

== Summary ==

Series 11298v1 drm/i915/dp/mst: Validate modes against the available link bandwidth
http://patchwork.freedesktop.org/api/1.0/series/11298/revisions/1/mbox

Test kms_cursor_legacy:
        Subgroup basic-flip-vs-cursor-varying-size:
                pass       -> FAIL       (ro-byt-n2820)
                pass       -> FAIL       (ro-skl3-i5-6260u)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)

ro-bdw-i5-5250u  total:240  pass:219  dwarn:3   dfail:0   fail:1   skip:17 
ro-bdw-i7-5557U  total:240  pass:220  dwarn:2   dfail:0   fail:0   skip:18 
ro-bdw-i7-5600u  total:240  pass:207  dwarn:0   dfail:0   fail:1   skip:32 
ro-bsw-n3050     total:240  pass:194  dwarn:0   dfail:0   fail:4   skip:42 
ro-byt-n2820     total:240  pass:197  dwarn:0   dfail:0   fail:3   skip:40 
ro-hsw-i3-4010u  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
ro-hsw-i7-4770r  total:240  pass:185  dwarn:0   dfail:0   fail:0   skip:55 
ro-ilk1-i5-650   total:235  pass:174  dwarn:0   dfail:0   fail:1   skip:60 
ro-ivb-i7-3770   total:240  pass:205  dwarn:0   dfail:0   fail:0   skip:35 
ro-ivb2-i7-3770  total:240  pass:209  dwarn:0   dfail:0   fail:0   skip:31 
ro-skl3-i5-6260u total:240  pass:222  dwarn:0   dfail:0   fail:4   skip:14 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1936/

9bc709c drm-intel-nightly: 2016y-08m-18d-16h-37m-38s UTC integration manifest
80e3da3 drm/i915/dp/mst: Validate modes against the available link bandwidth

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: failure for drm/i915/dp/mst: Validate modes against the available link bandwidth

[Srivatsa, Anusha]

________________________________________
From: Patchwork [patchwork@emeril.freedesktop.org]
Sent: Thursday, August 18, 2016 11:28 PM
To: Srivatsa, Anusha
Cc: intel-gfx@lists.freedesktop.org
Subject: ✗ Ro.CI.BAT: failure for drm/i915/dp/mst: Validate modes against the available link bandwidth

== Series Details ==

Series: drm/i915/dp/mst: Validate modes against the available link bandwidth
URL   : https://patchwork.freedesktop.org/series/11298/
State : failure

== Summary ==

Series 11298v1 drm/i915/dp/mst: Validate modes against the available link bandwidth
http://patchwork.freedesktop.org/api/1.0/series/11298/revisions/1/mbox

Test kms_cursor_legacy:
        Subgroup basic-flip-vs-cursor-varying-size:
                pass       -> FAIL       (ro-byt-n2820)
                pass       -> FAIL       (ro-skl3-i5-6260u)

https://bugs.freedesktop.org/show_bug.cgi?id=97188

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)

https://bugs.freedesktop.org/show_bug.cgi?id=96913


ro-bdw-i5-5250u  total:240  pass:219  dwarn:3   dfail:0   fail:1   skip:17
ro-bdw-i7-5557U  total:240  pass:220  dwarn:2   dfail:0   fail:0   skip:18
ro-bdw-i7-5600u  total:240  pass:207  dwarn:0   dfail:0   fail:1   skip:32
ro-bsw-n3050     total:240  pass:194  dwarn:0   dfail:0   fail:4   skip:42
ro-byt-n2820     total:240  pass:197  dwarn:0   dfail:0   fail:3   skip:40
ro-hsw-i3-4010u  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26
ro-hsw-i7-4770r  total:240  pass:185  dwarn:0   dfail:0   fail:0   skip:55
ro-ilk1-i5-650   total:235  pass:174  dwarn:0   dfail:0   fail:1   skip:60
ro-ivb-i7-3770   total:240  pass:205  dwarn:0   dfail:0   fail:0   skip:35
ro-ivb2-i7-3770  total:240  pass:209  dwarn:0   dfail:0   fail:0   skip:31
ro-skl3-i5-6260u total:240  pass:222  dwarn:0   dfail:0   fail:4   skip:14

Results at /archive/results/CI_IGT_test/RO_Patchwork_1936/

9bc709c drm-intel-nightly: 2016y-08m-18d-16h-37m-38s UTC integration manifest
80e3da3 drm/i915/dp/mst: Validate modes against the available link bandwidth

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

[Jim Bride]

On Thu, Aug 18, 2016 at 05:11:31PM -0700, Anusha Srivatsa wrote:
> Change intel_dp_mst_mode_valid() to use available link bandwidth
> rather than the link's maximum supported bandwidth to evaluate
> whether modes are legal for the current configuration. This takes
> into account the fact that link bandwidth may already be dedicated
> to other virtual channels.
> 
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>

Reviewed-by: Jim Bride <jim.bride@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 629337d..39c58eb 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -352,16 +352,22 @@ static enum drm_mode_status
>  intel_dp_mst_mode_valid(struct drm_connector *connector,
>  			struct drm_display_mode *mode)
>  {
> -	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> +	int req_pbn = 0;
> +	int slots = 0;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct intel_dp *intel_dp = intel_connector->mst_port;
> +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> +
> +	req_pbn = drm_dp_calc_pbn_mode(mode->clock, 24);
> +	slots = drm_dp_find_vcpi_slots(mgr, req_pbn);
>  
> -	/* TODO - validate mode against available PBN for link */
>  	if (mode->clock < 10000)
>  		return MODE_CLOCK_LOW;
>  
>  	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		return MODE_H_ILLEGAL;
>  
> -	if (mode->clock > max_dotclk)
> +	if (slots == -ENOSPC)
>  		return MODE_CLOCK_HIGH;
>  
>  	return MODE_OK;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

[Srivatsa, Anusha]

Sending to the list again since Ville's review comment  didn't hit the mailing list last time.

Regards,
Anusha

-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] 
Sent: Monday, September 5, 2016 4:39 AM
To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

On Thu, Aug 18, 2016 at 05:11:31PM -0700, Anusha Srivatsa wrote:
> Change intel_dp_mst_mode_valid() to use available link bandwidth 
> rather than the link's maximum supported bandwidth to evaluate whether 
> modes are legal for the current configuration. This takes into account 
> the fact that link bandwidth may already be dedicated to other virtual 
> channels.

We can't do this. Results of mode_valid() aren't supposed to change like this depending on what else is happening in the system. The only thing mode_valid() tells us is whether it's possible to use the mode under *some* circumstances. Only if the mode can't be used under *any* circumstances should it be filtered out.

If we wanted to change this, we'd at the very least have to synthesize hotplug uevents whenever the conditions changed. But doing that would be a fairly big behavioural change, so I'm not sure how people feel about it. It also doesn't really solve the problem since eg. atomic can go directly from 0->n active pipes, so there's no way to know upfront which modes we can pick for each pipe.

> 
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 629337d..39c58eb 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -352,16 +352,22 @@ static enum drm_mode_status  
> intel_dp_mst_mode_valid(struct drm_connector *connector,
>  			struct drm_display_mode *mode)
>  {
> -	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> +	int req_pbn = 0;
> +	int slots = 0;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct intel_dp *intel_dp = intel_connector->mst_port;
> +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> +
> +	req_pbn = drm_dp_calc_pbn_mode(mode->clock, 24);
> +	slots = drm_dp_find_vcpi_slots(mgr, req_pbn);
>  
> -	/* TODO - validate mode against available PBN for link */
>  	if (mode->clock < 10000)
>  		return MODE_CLOCK_LOW;
>  
>  	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		return MODE_H_ILLEGAL;
>  
> -	if (mode->clock > max_dotclk)
> +	if (slots == -ENOSPC)
>  		return MODE_CLOCK_HIGH;
>  
>  	return MODE_OK;
> --
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

[Jim Bride]

On Tue, Sep 06, 2016 at 06:39:12PM +0000, Srivatsa, Anusha wrote:
> Sending to the list again since Ville's review comment  didn't hit the mailing list last time.
> 
> Regards,
> Anusha
> 
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] 
> Sent: Monday, September 5, 2016 4:39 AM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth
> 
> On Thu, Aug 18, 2016 at 05:11:31PM -0700, Anusha Srivatsa wrote:
> > Change intel_dp_mst_mode_valid() to use available link bandwidth 
> > rather than the link's maximum supported bandwidth to evaluate whether 
> > modes are legal for the current configuration. This takes into account 
> > the fact that link bandwidth may already be dedicated to other virtual 
> > channels.
> 
> We can't do this. Results of mode_valid() aren't supposed to change like this depending on what else is happening in the system. The only thing mode_valid() tells us is whether it's possible to use the mode under *some* circumstances. Only if the mode can't be used under *any* circumstances should it be filtered out.
> 
> If we wanted to change this, we'd at the very least have to synthesize hotplug uevents whenever the conditions changed. But doing that would be a fairly big behavioural change, so I'm not sure how people feel about it. It also doesn't really solve the problem since eg. atomic can go directly from 0->n active pipes, so there's no way to know upfront which modes we can pick for each pipe.
> 

I won't dispute that this won't help for all cases, but it does make hotplugs,
at a minimum, more sane.  For that reason alone, I'd like to see this patch
land.  Longer term I think we should look at how we can make user space
and atomic better handle MST (IMHO in multi-modeset operations on MST atomic
should ensure that the aggregate dotclock of the modes being set are less
than the link bandwidth that's configured.)  I think that user space also
needs to be more MST aware and update what's available based on the current
configuration at any given time.  I'm not sure what the precise solution
should look like here, but I'd think that what we want is for user space
to fetch the list of available resolutions based on the current configuration
any time we plug or unplug a device on a DP MST topology.  In an ideal world
it would be nice to have some sort of information about the total link
bandwidth, and how much bandwidth each mode is taking up so that the user
could have a guide to tweaking their setup in such a way to maximize how
they choose to configure their monitors based on the available link
bandwidth.

Jim


> > 
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 629337d..39c58eb 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -352,16 +352,22 @@ static enum drm_mode_status  
> > intel_dp_mst_mode_valid(struct drm_connector *connector,
> >  			struct drm_display_mode *mode)
> >  {
> > -	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> > +	int req_pbn = 0;
> > +	int slots = 0;
> > +	struct intel_connector *intel_connector = to_intel_connector(connector);
> > +	struct intel_dp *intel_dp = intel_connector->mst_port;
> > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> > +
> > +	req_pbn = drm_dp_calc_pbn_mode(mode->clock, 24);
> > +	slots = drm_dp_find_vcpi_slots(mgr, req_pbn);
> >  
> > -	/* TODO - validate mode against available PBN for link */
> >  	if (mode->clock < 10000)
> >  		return MODE_CLOCK_LOW;
> >  
> >  	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> >  		return MODE_H_ILLEGAL;
> >  
> > -	if (mode->clock > max_dotclk)
> > +	if (slots == -ENOSPC)
> >  		return MODE_CLOCK_HIGH;
> >  
> >  	return MODE_OK;
> > --
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

[Srivatsa, Anusha]

Hi All,

Looking forward for more reviews comments and concerns if any, regarding this patch. 

Regards,
Anusha

>-----Original Message-----
>From: Jim Bride [mailto:jim.bride@linux.intel.com]
>Sent: Tuesday, September 6, 2016 11:57 AM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp/mst: Validate modes against the
>available link bandwidth
>
>On Tue, Sep 06, 2016 at 06:39:12PM +0000, Srivatsa, Anusha wrote:
>> Sending to the list again since Ville's review comment  didn't hit the mailing list
>last time.
>>
>> Regards,
>> Anusha
>>
>> -----Original Message-----
>> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> Sent: Monday, September 5, 2016 4:39 AM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp/mst: Validate modes
>> against the available link bandwidth
>>
>> On Thu, Aug 18, 2016 at 05:11:31PM -0700, Anusha Srivatsa wrote:
>> > Change intel_dp_mst_mode_valid() to use available link bandwidth
>> > rather than the link's maximum supported bandwidth to evaluate
>> > whether modes are legal for the current configuration. This takes
>> > into account the fact that link bandwidth may already be dedicated
>> > to other virtual channels.
>>
>> We can't do this. Results of mode_valid() aren't supposed to change like this
>depending on what else is happening in the system. The only thing mode_valid()
>tells us is whether it's possible to use the mode under *some* circumstances.
>Only if the mode can't be used under *any* circumstances should it be filtered
>out.
>>
>> If we wanted to change this, we'd at the very least have to synthesize hotplug
>uevents whenever the conditions changed. But doing that would be a fairly big
>behavioural change, so I'm not sure how people feel about it. It also doesn't
>really solve the problem since eg. atomic can go directly from 0->n active pipes,
>so there's no way to know upfront which modes we can pick for each pipe.
>>
>
>I won't dispute that this won't help for all cases, but it does make hotplugs, at a
>minimum, more sane.  For that reason alone, I'd like to see this patch land.
>Longer term I think we should look at how we can make user space and atomic
>better handle MST (IMHO in multi-modeset operations on MST atomic should
>ensure that the aggregate dotclock of the modes being set are less than the link
>bandwidth that's configured.)  I think that user space also needs to be more MST
>aware and update what's available based on the current configuration at any
>given time.  I'm not sure what the precise solution should look like here, but I'd
>think that what we want is for user space to fetch the list of available resolutions
>based on the current configuration any time we plug or unplug a device on a DP
>MST topology.  In an ideal world it would be nice to have some sort of
>information about the total link bandwidth, and how much bandwidth each mode
>is taking up so that the user could have a guide to tweaking their setup in such a
>way to maximize how they choose to configure their monitors based on the
>available link bandwidth.
>
>Jim
>
>
>> >
>> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_dp_mst.c | 12 +++++++++---
>> >  1 file changed, 9 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
>> > b/drivers/gpu/drm/i915/intel_dp_mst.c
>> > index 629337d..39c58eb 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>> > @@ -352,16 +352,22 @@ static enum drm_mode_status
>> > intel_dp_mst_mode_valid(struct drm_connector *connector,
>> >  			struct drm_display_mode *mode)
>> >  {
>> > -	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>> > +	int req_pbn = 0;
>> > +	int slots = 0;
>> > +	struct intel_connector *intel_connector =
>to_intel_connector(connector);
>> > +	struct intel_dp *intel_dp = intel_connector->mst_port;
>> > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
>> > +
>> > +	req_pbn = drm_dp_calc_pbn_mode(mode->clock, 24);
>> > +	slots = drm_dp_find_vcpi_slots(mgr, req_pbn);
>> >
>> > -	/* TODO - validate mode against available PBN for link */
>> >  	if (mode->clock < 10000)
>> >  		return MODE_CLOCK_LOW;
>> >
>> >  	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
>> >  		return MODE_H_ILLEGAL;
>> >
>> > -	if (mode->clock > max_dotclk)
>> > +	if (slots == -ENOSPC)
>> >  		return MODE_CLOCK_HIGH;
>> >
>> >  	return MODE_OK;
>> > --
>> > 2.7.4
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Ville Syrjälä
>> Intel OTC
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp/mst: Validate modes against the available link bandwidth

[Jani Nikula]

On Tue, 06 Sep 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> Ville wrote:
>> On Thu, Aug 18, 2016 at 05:11:31PM -0700, Anusha Srivatsa wrote:
>> > Change intel_dp_mst_mode_valid() to use available link bandwidth 
>> > rather than the link's maximum supported bandwidth to evaluate whether 
>> > modes are legal for the current configuration. This takes into account 
>> > the fact that link bandwidth may already be dedicated to other virtual 
>> > channels.
>> 
>> We can't do this. Results of mode_valid() aren't supposed to change
>> like this depending on what else is happening in the system. The only
>> thing mode_valid() tells us is whether it's possible to use the mode
>> under *some* circumstances. Only if the mode can't be used under
>> *any* circumstances should it be filtered out.
>> 
>> If we wanted to change this, we'd at the very least have to
>> synthesize hotplug uevents whenever the conditions changed. But doing
>> that would be a fairly big behavioural change, so I'm not sure how
>> people feel about it. It also doesn't really solve the problem since
>> eg. atomic can go directly from 0->n active pipes, so there's no way
>> to know upfront which modes we can pick for each pipe.
>> 
>
> I won't dispute that this won't help for all cases, but it does make hotplugs,
> at a minimum, more sane.  For that reason alone, I'd like to see this patch
> land.  Longer term I think we should look at how we can make user space
> and atomic better handle MST (IMHO in multi-modeset operations on MST atomic
> should ensure that the aggregate dotclock of the modes being set are less
> than the link bandwidth that's configured.)  I think that user space also
> needs to be more MST aware and update what's available based on the current
> configuration at any given time.  I'm not sure what the precise solution
> should look like here, but I'd think that what we want is for user space
> to fetch the list of available resolutions based on the current configuration
> any time we plug or unplug a device on a DP MST topology.  In an ideal world
> it would be nice to have some sort of information about the total link
> bandwidth, and how much bandwidth each mode is taking up so that the user
> could have a guide to tweaking their setup in such a way to maximize how
> they choose to configure their monitors based on the available link
> bandwidth.

We already have several restrictions outside of DP MST that impact what
configurations can be enabled at the same time. Things like available
PLLs, FDI bandwidth, etc. We can't prune the mode list on a first come,
first served basis. We can only look at the modes the userspace requests
at any given time, and tell the userspace whether we can deliver that or
not.

Imagine someone plugging in their 4k display, only to discover 1024x768
mode is there because all the bandwidth is reserved. With this patch,
they'd have to disable/unplug the first display, and then unplug/replug
the 4k display to see all the modes they have available.

BR,
Jani.


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