[Intel-gfx] [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 at linux.intel.com] 
Sent: Monday, September 5, 2016 4:39 AM
To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
Cc: intel-gfx at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC