Re: [PATCH 05/14] drm/sti: Try to fix up the tvout possible clones

Benjamin Gaignard
2018-06-18T08:16:16Z

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
>

--
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org