[Intel-gfx] [PATCH 09/16] drm/i915: Pimp HSW+ transcoder state readout
Navare, Manasi
manasi.d.navare at intel.com
Tue Sep 21 11:46:26 UTC 2021
On Mon, Sep 13, 2021 at 05:44:33PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Adjust the HSW+ transcoder state readout to just read through
> all the possible transcoders for the pipe, and stuff the results
> in a bitmask.
>
> We can conveniently cross check the bitmask for invalid
> combinations of enabled transcoders, and later we can easily
> extend the bitmask readout to handle the bigjoiner case.
>
> One slight change in behaviour is that we no longer read out
> the AONOFF->force_pfit.pfit bit for all the enabled "panel
> transcoders". But having more than one enabled would anyway
> be illegal so no big loss. Also the AONOFF selection should
> only ever be used on HSW, which only has the EDP transcoder
> an no DSI transcoders.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Looks good to me
Reviewed-by: Manasi Navare <manasi.d.navare at intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 130 ++++++++++++++-----
> 1 file changed, 95 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3848f7963cec..2430142b0337 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5577,6 +5577,21 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> return ret;
> }
>
> +static bool transcoder_ddi_func_is_enabled(struct drm_i915_private *dev_priv,
> + enum transcoder cpu_transcoder)
> +{
> + enum intel_display_power_domain power_domain;
> + intel_wakeref_t wakeref;
> + u32 tmp = 0;
> +
> + power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder);
> +
> + with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref)
> + tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
> +
> + return tmp & TRANS_DDI_FUNC_ENABLE;
> +}
> +
> static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
> {
> u8 panel_transcoder_mask = BIT(TRANSCODER_EDP);
> @@ -5587,55 +5602,39 @@ static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
> return panel_transcoder_mask;
> }
>
> -static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
> - struct intel_crtc_state *pipe_config,
> - struct intel_display_power_domain_set *power_domain_set)
> +static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> u8 panel_transcoder_mask = hsw_panel_transcoders(dev_priv);
> - unsigned long enabled_panel_transcoders = 0;
> - enum transcoder panel_transcoder;
> - u32 tmp;
> -
> - /*
> - * The pipe->transcoder mapping is fixed with the exception of the eDP
> - * and DSI transcoders handled below.
> - */
> - pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> + enum transcoder cpu_transcoder;
> + u8 enabled_transcoders = 0;
>
> /*
> * XXX: Do intel_display_power_get_if_enabled before reading this (for
> * consistency and less surprising code; it's in always on power).
> */
> - for_each_cpu_transcoder_masked(dev_priv, panel_transcoder,
> + for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> panel_transcoder_mask) {
> - bool force_thru = false;
> + enum intel_display_power_domain power_domain;
> + intel_wakeref_t wakeref;
> enum pipe trans_pipe;
> + u32 tmp = 0;
> +
> + power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder);
> + with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref)
> + tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
>
> - tmp = intel_de_read(dev_priv,
> - TRANS_DDI_FUNC_CTL(panel_transcoder));
> if (!(tmp & TRANS_DDI_FUNC_ENABLE))
> continue;
>
> - /*
> - * Log all enabled ones, only use the first one.
> - *
> - * FIXME: This won't work for two separate DSI displays.
> - */
> - enabled_panel_transcoders |= BIT(panel_transcoder);
> - if (enabled_panel_transcoders != BIT(panel_transcoder))
> - continue;
> -
> switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
> default:
> drm_WARN(dev, 1,
> "unknown pipe linked to transcoder %s\n",
> - transcoder_name(panel_transcoder));
> + transcoder_name(cpu_transcoder));
> fallthrough;
> case TRANS_DDI_EDP_INPUT_A_ONOFF:
> - force_thru = true;
> - fallthrough;
> case TRANS_DDI_EDP_INPUT_A_ON:
> trans_pipe = PIPE_A;
> break;
> @@ -5650,22 +5649,83 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
> break;
> }
>
> - if (trans_pipe == crtc->pipe) {
> - pipe_config->cpu_transcoder = panel_transcoder;
> - pipe_config->pch_pfit.force_thru = force_thru;
> - }
> + if (trans_pipe == crtc->pipe)
> + enabled_transcoders |= BIT(cpu_transcoder);
> }
>
> + cpu_transcoder = (enum transcoder) crtc->pipe;
> + if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
> + enabled_transcoders |= BIT(cpu_transcoder);
> +
> + return enabled_transcoders;
> +}
> +
> +static bool has_edp_transcoders(u8 enabled_transcoders)
> +{
> + return enabled_transcoders & BIT(TRANSCODER_EDP);
> +}
> +
> +static bool has_dsi_transcoders(u8 enabled_transcoders)
> +{
> + return enabled_transcoders & (BIT(TRANSCODER_DSI_0) |
> + BIT(TRANSCODER_DSI_1));
> +}
> +
> +static bool has_pipe_transcoders(u8 enabled_transcoders)
> +{
> + return enabled_transcoders & ~(BIT(TRANSCODER_EDP) |
> + BIT(TRANSCODER_DSI_0) |
> + BIT(TRANSCODER_DSI_1));
> +}
> +
> +static void assert_enabled_transcoders(struct drm_i915_private *i915,
> + u8 enabled_transcoders)
> +{
> + /* Only one type of transcoder please */
> + drm_WARN_ON(&i915->drm,
> + has_edp_transcoders(enabled_transcoders) +
> + has_dsi_transcoders(enabled_transcoders) +
> + has_pipe_transcoders(enabled_transcoders) > 1);
> +
> + /* Only DSI transcoders can be ganged */
> + drm_WARN_ON(&i915->drm,
> + !has_dsi_transcoders(enabled_transcoders) &&
> + !is_power_of_2(enabled_transcoders));
> +}
> +
> +static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
> + struct intel_crtc_state *pipe_config,
> + struct intel_display_power_domain_set *power_domain_set)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + unsigned long enabled_transcoders;
> + u32 tmp;
> +
> + enabled_transcoders = hsw_enabled_transcoders(crtc);
> + if (!enabled_transcoders)
> + return false;
> +
> + assert_enabled_transcoders(dev_priv, enabled_transcoders);
> +
> /*
> - * Valid combos: none, eDP, DSI0, DSI1, DSI0+DSI1
> + * With the exception of DSI we should only ever have
> + * a single enabled transcoder. With DSI let's just
> + * pick the first one.
> */
> - drm_WARN_ON(dev, (enabled_panel_transcoders & BIT(TRANSCODER_EDP)) &&
> - enabled_panel_transcoders != BIT(TRANSCODER_EDP));
> + pipe_config->cpu_transcoder = ffs(enabled_transcoders) - 1;
>
> if (!intel_display_power_get_in_set_if_enabled(dev_priv, power_domain_set,
> POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
> return false;
>
> + if (hsw_panel_transcoders(dev_priv) & BIT(pipe_config->cpu_transcoder)) {
> + tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(pipe_config->cpu_transcoder));
> +
> + if ((tmp & TRANS_DDI_EDP_INPUT_MASK) == TRANS_DDI_EDP_INPUT_A_ONOFF)
> + pipe_config->pch_pfit.force_thru = true;
> + }
> +
> tmp = intel_de_read(dev_priv, PIPECONF(pipe_config->cpu_transcoder));
>
> return tmp & PIPECONF_ENABLE;
> --
> 2.32.0
>
More information about the Intel-gfx
mailing list