All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anshuman Gupta <anshuman.gupta@intel.com>
To: Uma Shankar <uma.shankar@intel.com>
Cc: kai.vehmanen@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [v5] drm/i915/display: Enable DP Display Audio WA
Date: Thu, 16 Apr 2020 15:28:28 +0530	[thread overview]
Message-ID: <20200416095828.GK5533@intel.com> (raw)
In-Reply-To: <20200416074721.472-1-uma.shankar@intel.com>

On 2020-04-16 at 13:17:21 +0530, Uma Shankar wrote:
> For certain DP VDSC bpp settings, hblank asserts before hblank_early,
> leading to a bad audio state. Driver need to program "hblank early
> enable" and "samples per line" parameters in AUDIO_CONFIG_BE
> register.
> 
> This is Display Audio WA #1406928334 for 4k+VDSC usecase
> applicable on DP encoders. Implemented the same.
> 
> v2: Fixed build failures on 32bit machine.
> 
> v3: Dropped u64, added helpers for sample room calculation,
>     other general comments as per Jani Nikula's feedback.
>     Also fixed connector type check (spotted by Anshuman)
> 
> v4: Addressed Jani Nikula and Kai's review comments.
> 
> v5: Addressed Anshuman's review comment and used crtc_* variable
>     to get timings.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 147 +++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h            |  16 +++
>  2 files changed, 163 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 57b80971ae78..ca678da1152f 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -514,6 +514,149 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
>  	mutex_unlock(&dev_priv->av_mutex);
>  }
>  
> +/* Add a factor to take care of rounding and truncations */
> +#define ROUNDING_FACTOR 10000
> +
> +static unsigned int get_hblank_early_enable_config(struct intel_encoder *encoder,
> +						   const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	unsigned int link_clks_available, link_clks_required;
> +	unsigned int tu_data, tu_line, link_clks_active;
> +	unsigned int hblank_rise, hblank_early_prog;
> +	unsigned int h_active, h_total, hblank_delta, pixel_clk, v_total;
> +	unsigned int fec_coeff, refresh_rate, cdclk, vdsc_bpp;
> +
> +	h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> +	h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> +	v_total = crtc_state->hw.adjusted_mode.crtc_vtotal;
> +	hblank_rise = crtc_state->hw.adjusted_mode.crtc_hsync_start;
	IMHO with control flow this function above initilization is not required.
	u may remove this initilization if u agree, while pushing the patch. 
	Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> +	pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock;
> +	refresh_rate = crtc_state->hw.adjusted_mode.vrefresh;
> +	vdsc_bpp = crtc_state->dsc.compressed_bpp;
> +	cdclk = i915->cdclk.hw.cdclk;
> +	/* fec= 0.972261, using rounding multiplier of 1000000 */
> +	fec_coeff = 972261;
> +
> +	drm_dbg_kms(&i915->drm, "h_active = %u link_clk = %u :"
> +		    "lanes = %u vdsc_bpp = %u cdclk = %u\n",
> +		    h_active, crtc_state->port_clock, crtc_state->lane_count,
> +		    vdsc_bpp, cdclk);
> +
> +	link_clks_available = ((((h_total - h_active) *
> +			       ((crtc_state->port_clock * ROUNDING_FACTOR) /
> +				pixel_clk)) / ROUNDING_FACTOR) - 28);
> +
> +	link_clks_required = DIV_ROUND_UP(192000, (refresh_rate *
> +					  v_total)) * ((48 /
> +					  crtc_state->lane_count) + 2);
> +
> +	if (link_clks_available > link_clks_required)
> +		hblank_delta = 32;
> +	else
> +		hblank_delta = DIV_ROUND_UP(((((5 * ROUNDING_FACTOR) /
> +					    crtc_state->port_clock) + ((5 *
> +					    ROUNDING_FACTOR) /
> +					    cdclk)) * pixel_clk),
> +					    ROUNDING_FACTOR);
> +
> +	tu_data = (pixel_clk * vdsc_bpp * 8) / ((crtc_state->port_clock *
> +		   crtc_state->lane_count * fec_coeff) / 1000000);
> +	tu_line = (((h_active * crtc_state->port_clock * fec_coeff) /
> +		   1000000) / (64 * pixel_clk));
> +	link_clks_active  = (tu_line - 1) * 64 + tu_data;
> +
> +	hblank_rise = ((link_clks_active + 6 * DIV_ROUND_UP(link_clks_active,
> +			250) + 4) * ((pixel_clk * ROUNDING_FACTOR) /
> +			crtc_state->port_clock)) / ROUNDING_FACTOR;
> +
> +	hblank_early_prog = h_active - hblank_rise + hblank_delta;
> +
> +	return hblank_early_prog;
> +}
> +
> +static unsigned int get_sample_room_req_config(const struct intel_crtc_state *crtc_state)
> +{
> +	unsigned int h_active, h_total, pixel_clk;
> +	unsigned int samples_room;
> +
> +	h_active = crtc_state->hw.adjusted_mode.hdisplay;
> +	h_total = crtc_state->hw.adjusted_mode.htotal;
> +	pixel_clk = crtc_state->hw.adjusted_mode.clock;
> +
> +	samples_room = ((((h_total - h_active) * ((crtc_state->port_clock *
> +			ROUNDING_FACTOR) / pixel_clk)) /
> +			ROUNDING_FACTOR) - 12) / ((48 /
> +			crtc_state->lane_count) + 2);
> +
> +	return samples_room;
> +}
> +
> +static void enable_audio_dsc_wa(struct intel_encoder *encoder,
> +				const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	unsigned int hblank_early_prog, samples_room, h_active;
> +	unsigned int val;
> +
> +	if (INTEL_GEN(i915) < 11)
> +		return;
> +
> +	h_active = crtc_state->hw.adjusted_mode.hdisplay;
> +
> +	if (!(h_active && crtc_state->port_clock && crtc_state->lane_count &&
> +	      crtc_state->dsc.compressed_bpp && i915->cdclk.hw.cdclk)) {
> +		drm_err(&i915->drm, "Null Params rcvd for hblank early enabling\n");
> +		WARN_ON(1);
> +		return;
> +	}
> +
> +	val = intel_de_read(i915, AUD_CONFIG_BE);
> +
> +	if (INTEL_GEN(i915) == 11)
> +		val |= HBLANK_EARLY_ENABLE_ICL(pipe);
> +	else if (INTEL_GEN(i915) >= 12)
> +		val |= HBLANK_EARLY_ENABLE_TGL(pipe);
> +
> +	if (crtc_state->dsc.compression_enable &&
> +	    (crtc_state->hw.adjusted_mode.hdisplay >= 3840 &&
> +	    crtc_state->hw.adjusted_mode.vdisplay >= 2160)) {
> +		/* Get hblank early enable value required */
> +		hblank_early_prog = get_hblank_early_enable_config(encoder,
> +								   crtc_state);
> +		if (hblank_early_prog < 32) {
> +			val &= ~HBLANK_START_COUNT_MASK(pipe);
> +			val |= HBLANK_START_COUNT(pipe, HBLANK_START_COUNT_32);
> +		} else if (hblank_early_prog < 64) {
> +			val &= ~HBLANK_START_COUNT_MASK(pipe);
> +			val |= HBLANK_START_COUNT(pipe, HBLANK_START_COUNT_64);
> +		} else if (hblank_early_prog < 96) {
> +			val &= ~HBLANK_START_COUNT_MASK(pipe);
> +			val |= HBLANK_START_COUNT(pipe, HBLANK_START_COUNT_96);
> +		} else {
> +			val &= ~HBLANK_START_COUNT_MASK(pipe);
> +			val |= HBLANK_START_COUNT(pipe, HBLANK_START_COUNT_128);
> +		}
> +
> +		/* Get samples room value required */
> +		samples_room = get_sample_room_req_config(crtc_state);
> +		if (samples_room < 3) {
> +			val &= ~NUMBER_SAMPLES_PER_LINE_MASK(pipe);
> +			val |= NUMBER_SAMPLES_PER_LINE(pipe, samples_room);
> +		} else {
> +			/* Program 0 i.e "All Samples available in buffer" */
> +			val &= ~NUMBER_SAMPLES_PER_LINE_MASK(pipe);
> +			val |= NUMBER_SAMPLES_PER_LINE(pipe, 0x0);
> +		}
> +	}
> +
> +	intel_de_write(i915, AUD_CONFIG_BE, val);
> +}
> +
> +#undef ROUNDING_FACTOR
> +
>  static void hsw_audio_codec_enable(struct intel_encoder *encoder,
>  				   const struct intel_crtc_state *crtc_state,
>  				   const struct drm_connector_state *conn_state)
> @@ -531,6 +674,10 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/* Enable Audio WA for 4k DSC usecases */
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
> +		enable_audio_dsc_wa(encoder, crtc_state);
> +
>  	/* Enable audio presence detect, invalidate ELD */
>  	tmp = intel_de_read(dev_priv, HSW_AUD_PIN_ELD_CP_VLD);
>  	tmp |= AUDIO_OUTPUT_ENABLE(cpu_transcoder);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0b39b9abf8a4..8ce79ec53bad 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9396,6 +9396,22 @@ enum {
>  #define AUD_PIN_BUF_CTL		_MMIO(0x48414)
>  #define   AUD_PIN_BUF_ENABLE		REG_BIT(31)
>  
> +/* Display Audio Config Reg */
> +#define AUD_CONFIG_BE			_MMIO(0x65ef0)
> +#define HBLANK_EARLY_ENABLE_ICL(pipe)		(0x1 << (20 - (pipe)))
> +#define HBLANK_EARLY_ENABLE_TGL(pipe)		(0x1 << (24 + (pipe)))
> +#define HBLANK_START_COUNT_MASK(pipe)		(0x7 << (3 + ((pipe) * 6)))
> +#define HBLANK_START_COUNT(pipe, val)		(((val) & 0x7) << (3 + ((pipe)) * 6))
> +#define NUMBER_SAMPLES_PER_LINE_MASK(pipe)	(0x3 << ((pipe) * 6))
> +#define NUMBER_SAMPLES_PER_LINE(pipe, val)	(((val) & 0x3) << ((pipe) * 6))
> +
> +#define HBLANK_START_COUNT_8	0
> +#define HBLANK_START_COUNT_16	1
> +#define HBLANK_START_COUNT_32	2
> +#define HBLANK_START_COUNT_64	3
> +#define HBLANK_START_COUNT_96	4
> +#define HBLANK_START_COUNT_128	5
> +
>  /*
>   * HSW - ICL power wells
>   *
> -- 
> 2.22.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2020-04-16 10:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-16  7:47 [Intel-gfx] [v5] drm/i915/display: Enable DP Display Audio WA Uma Shankar
2020-04-16  8:13 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/display: Enable DP Display Audio WA (rev5) Patchwork
2020-04-16  8:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-04-16  9:58 ` Anshuman Gupta [this message]
2020-04-16 10:13   ` [Intel-gfx] [v5] drm/i915/display: Enable DP Display Audio WA Jani Nikula
2020-04-16 10:16     ` Shankar, Uma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200416095828.GK5533@intel.com \
    --to=anshuman.gupta@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=kai.vehmanen@intel.com \
    --cc=uma.shankar@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.