| Message ID | 20180615164925.28971-6-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2018-06-15 18:49 GMT+02:00 Ville Syrjala <ville.syrjala@linux.intel.com>: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The current possible_clones setup doesn't look sensible. I'm assuming > the 0 and 1 are supposed to refer to the indexes of the hdmi and hda > encoders? So it kinda looks like we want hda+hdmi cloning, but then > dvo also claims to be cloneable with hdmi, but hdmi won't recipricate. All cloning combinaisons are possible: hdmi+hda, hda+dvo, hdmi+dvo. The correct solution should be: possible_clones = drm_encoder_mask(tvout->hdmi) | drm_encoder_mask(tvout->hda) | drm_encoder_mask(tvout->dvo); Thanks, Benjamin > > So let's assume we just want hdmi+hda and nothing more and populate the > possible_clones to match. The docs say the encoder itself should always > be included in the possible_clones bitmask so we'll throw it in there > as well. For hda we'll leave possible_clones==0 and we'll add a global > fallback for that case to add the encoder itself to the bitmask since > that makes life easier for most drivers. > > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org> > Cc: Vincent Abriou <vincent.abriou@st.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/sti/sti_tvout.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c > index 7d495307fe79..2359c111c7ed 100644 > --- a/drivers/gpu/drm/sti/sti_tvout.c > +++ b/drivers/gpu/drm/sti/sti_tvout.c > @@ -668,7 +668,6 @@ sti_tvout_create_dvo_encoder(struct drm_device *dev, > drm_encoder = &encoder->encoder; > > drm_encoder->possible_crtcs = ENCODER_CRTC_MASK; > - drm_encoder->possible_clones = 1 << 0; > > drm_encoder_init(dev, drm_encoder, > &sti_tvout_encoder_funcs, DRM_MODE_ENCODER_LVDS, > @@ -721,7 +720,6 @@ static struct drm_encoder *sti_tvout_create_hda_encoder(struct drm_device *dev, > drm_encoder = &encoder->encoder; > > drm_encoder->possible_crtcs = ENCODER_CRTC_MASK; > - drm_encoder->possible_clones = 1 << 0; > > drm_encoder_init(dev, drm_encoder, > &sti_tvout_encoder_funcs, DRM_MODE_ENCODER_DAC, NULL); > @@ -770,7 +768,6 @@ static struct drm_encoder *sti_tvout_create_hdmi_encoder(struct drm_device *dev, > drm_encoder = &encoder->encoder; > > drm_encoder->possible_crtcs = ENCODER_CRTC_MASK; > - drm_encoder->possible_clones = 1 << 1; > > drm_encoder_init(dev, drm_encoder, > &sti_tvout_encoder_funcs, DRM_MODE_ENCODER_TMDS, NULL); > @@ -786,6 +783,12 @@ static void sti_tvout_create_encoders(struct drm_device *dev, > tvout->hdmi = sti_tvout_create_hdmi_encoder(dev, tvout); > tvout->hda = sti_tvout_create_hda_encoder(dev, tvout); > tvout->dvo = sti_tvout_create_dvo_encoder(dev, tvout); > + > + /* FIXME what's the correct thing here? */ > + tvout->hdmi->possible_clones = > + drm_encoder_mask(tvout->hdmi) | drm_encoder_mask(tvout->hda); > + tvout->hda->possible_clones = > + drm_encoder_mask(tvout->hdmi) | drm_encoder_mask(tvout->hda); > } > > static void sti_tvout_destroy_encoders(struct sti_tvout *tvout) > -- > 2.16.4 >
diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c index 7d495307fe79..2359c111c7ed 100644 --- a/drivers/gpu/drm/sti/sti_tvout.c +++ b/drivers/gpu/drm/sti/sti_tvout.c @@ -668,7 +668,6 @@ sti_tvout_create_dvo_encoder(struct drm_device *dev, drm_encoder = &encoder->encoder; drm_encoder->possible_crtcs = ENCODER_CRTC_MASK; - drm_encoder->possible_clones = 1 << 0; drm_encoder_init(dev, drm_encoder, &sti_tvout_encoder_funcs, DRM_MODE_ENCODER_LVDS, @@ -721,7 +720,6 @@ static struct drm_encoder *sti_tvout_create_hda_encoder(struct drm_device *dev, drm_encoder = &encoder->encoder; drm_encoder->possible_crtcs = ENCODER_CRTC_MASK; - drm_encoder->possible_clones = 1 << 0; drm_encoder_init(dev, drm_encoder, &sti_tvout_encoder_funcs, DRM_MODE_ENCODER_DAC, NULL); @@ -770,7 +768,6 @@ static struct drm_encoder *sti_tvout_create_hdmi_encoder(struct drm_device *dev, drm_encoder = &encoder->encoder; drm_encoder->possible_crtcs = ENCODER_CRTC_MASK; - drm_encoder->possible_clones = 1 << 1; drm_encoder_init(dev, drm_encoder, &sti_tvout_encoder_funcs, DRM_MODE_ENCODER_TMDS, NULL); @@ -786,6 +783,12 @@ static void sti_tvout_create_encoders(struct drm_device *dev, tvout->hdmi = sti_tvout_create_hdmi_encoder(dev, tvout); tvout->hda = sti_tvout_create_hda_encoder(dev, tvout); tvout->dvo = sti_tvout_create_dvo_encoder(dev, tvout); + + /* FIXME what's the correct thing here? */ + tvout->hdmi->possible_clones = + drm_encoder_mask(tvout->hdmi) | drm_encoder_mask(tvout->hda); + tvout->hda->possible_clones = + drm_encoder_mask(tvout->hdmi) | drm_encoder_mask(tvout->hda); } static void sti_tvout_destroy_encoders(struct sti_tvout *tvout)