Re: [PATCH 6/6] drm/i915/mst: Document the userspace fail with possible_crtcs

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 02, 2019 at 07:25:05PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> To avoid accidentally breaking things in the future add a
> comment explaining why we misconfigure the pipe_mask.
> 
> Also toss in a TODO for investigating a single encoder
> approach as opposed to the encoder-per-pipe approach.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 7be82cf926ca..cb3047fe2d02 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -615,6 +615,18 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
>  	intel_encoder->power_domain = intel_dig_port->base.power_domain;
>  	intel_encoder->port = intel_dig_port->base.port;
>  	intel_encoder->cloneable = 0;
> +	/*
> +	 * This is wrong, but broken userspace uses the intersection
> +	 * of possible_crtcs of all the encoders of a given connector
> +	 * to figure out which crtcs can drive said connector. What
> +	 * should be used instead is the union of possible_crtcs.
> +	 * To keep such userspace functioning we must misconfigure
> +	 * this to make sure the intersection is not empty :(
> +	 *
> +	 * TODO: figure out if we could eliminate the per-pipe
> +	 * encoders here and just have a single encoder for each
> +	 * MST connector. That would sidestep the userspace bug.

That of course won't work since we can't register encoders dynamically.
I guess we'll just have to live with this slight discrepancy with the
possible_crtcs. We could make the encoder<->pipe assignment totally
flexible but that wouldn't actually change anything. We still get to
pick one based on whatever reason we can think of, and using the pipe
for that seems as good a reason as any.

What we could try to remove is having separate MST encoders for each
DDI port. But that would make encoder->port not work for MST again. So
defintiely has downsides, and doens't do anything for the possible_crtcs
thing.

Bah. I'll just drop the TODO.

> +	 */
>  	intel_encoder->pipe_mask = ~0;
>  
>  	intel_encoder->compute_config = intel_dp_mst_compute_config;
> -- 
> 2.21.0

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