| Message ID | 20191002162505.30716-6-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] drm/i915: Polish possible_clones setup | expand |
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
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. + */ intel_encoder->pipe_mask = ~0; intel_encoder->compute_config = intel_dp_mst_compute_config;