Re: [PATCH 08/13] drm/i915/dsb: Handle the indexed vs. not inside the DSB code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: Friday, December 16, 2022 6:08 AM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject:  [PATCH 08/13] drm/i915/dsb: Handle the indexed vs. not
> inside the DSB code
> 
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> The DSB indexed register write insturction is purely an internal DSB
> implementation detail, no reason why the caller should have to know about
> it. So let's just have the caller emit blind register writes let the DSB code
> convert things to an indexed write if/when multiple writes occur to the same
> register offset in a row.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

LGTM.
Reviewed-by: Animesh Manna <animesh.manna@xxxxxxxxx>

> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 45 ++++------
>  drivers/gpu/drm/i915/display/intel_dsb.c   | 96 +++++++++-------------
>  drivers/gpu/drm/i915/display/intel_dsb.h   |  2 -
>  3 files changed, 54 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index d57631b0bb9a..76603357252d 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -847,17 +847,6 @@ static void ilk_lut_write(const struct intel_crtc_state
> *crtc_state,
>  		intel_de_write_fw(i915, reg, val);
>  }
> 
> -static void ilk_lut_write_indexed(const struct intel_crtc_state *crtc_state,
> -				  i915_reg_t reg, u32 val)
> -{
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> -
> -	if (crtc_state->dsb)
> -		intel_dsb_indexed_reg_write(crtc_state->dsb, reg, val);
> -	else
> -		intel_de_write_fw(i915, reg, val);
> -}
> -
>  static void ilk_load_lut_8(const struct intel_crtc_state *crtc_state,
>  			   const struct drm_property_blob *blob)  { @@ -
> 962,8 +951,8 @@ static void bdw_load_lut_10(const struct intel_crtc_state
> *crtc_state,
>  		      prec_index);
> 
>  	for (i = 0; i < lut_size; i++)
> -		ilk_lut_write_indexed(crtc_state, PREC_PAL_DATA(pipe),
> -				      ilk_lut_10(&lut[i]));
> +		ilk_lut_write(crtc_state, PREC_PAL_DATA(pipe),
> +			      ilk_lut_10(&lut[i]));
> 
>  	/*
>  	 * Reset the index, otherwise it prevents the legacy palette to be @@
> -1093,13 +1082,13 @@ static void glk_load_degamma_lut(const struct
> intel_crtc_state *crtc_state,
>  		 * ToDo: Extend to max 7.0. Enable 32 bit input value
>  		 * as compared to just 16 to achieve this.
>  		 */
> -		ilk_lut_write_indexed(crtc_state,
> PRE_CSC_GAMC_DATA(pipe),
> -				      lut[i].green);
> +		ilk_lut_write(crtc_state, PRE_CSC_GAMC_DATA(pipe),
> +			      lut[i].green);
>  	}
> 
>  	/* Clamp values > 1.0. */
>  	while (i++ < glk_degamma_lut_size(i915))
> -		ilk_lut_write_indexed(crtc_state,
> PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> +		ilk_lut_write(crtc_state, PRE_CSC_GAMC_DATA(pipe), 1 <<
> 16);
> 
>  	ilk_lut_write(crtc_state, PRE_CSC_GAMC_INDEX(pipe), 0);  } @@ -
> 1165,10 +1154,10 @@ icl_program_gamma_superfine_segment(const struct
> intel_crtc_state *crtc_state)
>  	for (i = 0; i < 9; i++) {
>  		const struct drm_color_lut *entry = &lut[i];
> 
> -		ilk_lut_write_indexed(crtc_state,
> PREC_PAL_MULTI_SEG_DATA(pipe),
> -				      ilk_lut_12p4_ldw(entry));
> -		ilk_lut_write_indexed(crtc_state,
> PREC_PAL_MULTI_SEG_DATA(pipe),
> -				      ilk_lut_12p4_udw(entry));
> +		ilk_lut_write(crtc_state, PREC_PAL_MULTI_SEG_DATA(pipe),
> +			      ilk_lut_12p4_ldw(entry));
> +		ilk_lut_write(crtc_state, PREC_PAL_MULTI_SEG_DATA(pipe),
> +			      ilk_lut_12p4_udw(entry));
>  	}
> 
>  	ilk_lut_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe), @@ -
> 1204,10 +1193,10 @@ icl_program_gamma_multi_segment(const struct
> intel_crtc_state *crtc_state)
>  	for (i = 1; i < 257; i++) {
>  		entry = &lut[i * 8];
> 
> -		ilk_lut_write_indexed(crtc_state, PREC_PAL_DATA(pipe),
> -				      ilk_lut_12p4_ldw(entry));
> -		ilk_lut_write_indexed(crtc_state, PREC_PAL_DATA(pipe),
> -				      ilk_lut_12p4_udw(entry));
> +		ilk_lut_write(crtc_state, PREC_PAL_DATA(pipe),
> +			      ilk_lut_12p4_ldw(entry));
> +		ilk_lut_write(crtc_state, PREC_PAL_DATA(pipe),
> +			      ilk_lut_12p4_udw(entry));
>  	}
> 
>  	/*
> @@ -1225,10 +1214,10 @@ icl_program_gamma_multi_segment(const
> struct intel_crtc_state *crtc_state)
>  	for (i = 0; i < 256; i++) {
>  		entry = &lut[i * 8 * 128];
> 
> -		ilk_lut_write_indexed(crtc_state, PREC_PAL_DATA(pipe),
> -				      ilk_lut_12p4_ldw(entry));
> -		ilk_lut_write_indexed(crtc_state, PREC_PAL_DATA(pipe),
> -				      ilk_lut_12p4_udw(entry));
> +		ilk_lut_write(crtc_state, PREC_PAL_DATA(pipe),
> +			      ilk_lut_12p4_ldw(entry));
> +		ilk_lut_write(crtc_state, PREC_PAL_DATA(pipe),
> +			      ilk_lut_12p4_udw(entry));
>  	}
> 
>  	ilk_lut_write(crtc_state, PREC_PAL_INDEX(pipe), diff --git
> a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index fcc3f49c5445..fa4b808a8134 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -114,32 +114,28 @@ static bool intel_dsb_prev_ins_is_write(struct
> intel_dsb *dsb,
>  	return prev_opcode == opcode && prev_reg ==
> i915_mmio_reg_offset(reg);  }
> 
> +static bool intel_dsb_prev_ins_is_mmio_write(struct intel_dsb *dsb,
> +i915_reg_t reg) {
> +	return intel_dsb_prev_ins_is_write(dsb,
> DSB_OPCODE_MMIO_WRITE, reg); }
> +
>  static bool intel_dsb_prev_ins_is_indexed_write(struct intel_dsb *dsb,
> i915_reg_t reg)  {
>  	return intel_dsb_prev_ins_is_write(dsb,
> DSB_OPCODE_INDEXED_WRITE, reg);  }
> 
>  /**
> - * intel_dsb_indexed_reg_write() -Write to the DSB context for auto
> - * increment register.
> + * intel_dsb_reg_write() - Emit register wriite to the DSB context
>   * @dsb: DSB context
>   * @reg: register address.
>   * @val: value.
>   *
>   * This function is used for writing register-value pair in command
> - * buffer of DSB for auto-increment register. During command buffer
> overflow,
> - * a warning is thrown and rest all erroneous condition register
> programming
> - * is done through mmio write.
> + * buffer of DSB.
>   */
> -
> -void intel_dsb_indexed_reg_write(struct intel_dsb *dsb,
> -				 i915_reg_t reg, u32 val)
> +void intel_dsb_reg_write(struct intel_dsb *dsb,
> +			 i915_reg_t reg, u32 val)
>  {
> -	u32 *buf = dsb->cmd_buf;
> -
> -	if (!assert_dsb_has_room(dsb))
> -		return;
> -
>  	/*
>  	 * For example the buffer will look like below for 3 dwords for auto
>  	 * increment register:
> @@ -156,57 +152,39 @@ void intel_dsb_indexed_reg_write(struct intel_dsb
> *dsb,
>  	 * we are writing odd no of dwords, Zeros will be added in the end
> for
>  	 * padding.
>  	 */
> -	if (!intel_dsb_prev_ins_is_indexed_write(dsb, reg)) {
> -		/* Every instruction should be 8 byte aligned. */
> -		dsb->free_pos = ALIGN(dsb->free_pos, 2);
> -
> -		dsb->ins_start_offset = dsb->free_pos;
> -
> -		/* Update the size. */
> -		buf[dsb->free_pos++] = 1;
> -
> -		/* Update the opcode and reg. */
> -		buf[dsb->free_pos++] = (DSB_OPCODE_INDEXED_WRITE  <<
> -					DSB_OPCODE_SHIFT) |
> -					i915_mmio_reg_offset(reg);
> -
> -		/* Update the value. */
> -		buf[dsb->free_pos++] = val;
> +	if (!intel_dsb_prev_ins_is_mmio_write(dsb, reg) &&
> +	    !intel_dsb_prev_ins_is_indexed_write(dsb, reg)) {
> +		intel_dsb_emit(dsb, val,
> +			       (DSB_OPCODE_MMIO_WRITE <<
> DSB_OPCODE_SHIFT) |
> +			       (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
> +			       i915_mmio_reg_offset(reg));
>  	} else {
> -		/* Update the new value. */
> +		u32 *buf = dsb->cmd_buf;
> +
> +		if (!assert_dsb_has_room(dsb))
> +			return;
> +
> +		/* convert to indexed write? */
> +		if (intel_dsb_prev_ins_is_mmio_write(dsb, reg)) {
> +			u32 prev_val = buf[dsb->ins_start_offset + 0];
> +
> +			buf[dsb->ins_start_offset + 0] = 1; /* size */
> +			buf[dsb->ins_start_offset + 1] =
> +				(DSB_OPCODE_INDEXED_WRITE <<
> DSB_OPCODE_SHIFT) |
> +				i915_mmio_reg_offset(reg);
> +			buf[dsb->ins_start_offset + 2] = prev_val;
> +
> +			dsb->free_pos++;
> +		}
> +
>  		buf[dsb->free_pos++] = val;
> -
>  		/* Update the size. */
>  		buf[dsb->ins_start_offset]++;
> +
> +		/* if number of data words is odd, then the last dword
> should be 0.*/
> +		if (dsb->free_pos & 0x1)
> +			buf[dsb->free_pos] = 0;
>  	}
> -
> -	/* if number of data words is odd, then the last dword should be
> 0.*/
> -	if (dsb->free_pos & 0x1)
> -		buf[dsb->free_pos] = 0;
> -}
> -
> -/**
> - * intel_dsb_reg_write() -Write to the DSB context for normal
> - * register.
> - * @crtc_state: intel_crtc_state structure
> - * @reg: register address.
> - * @val: value.
> - *
> - * This function is used for writing register-value pair in command
> - * buffer of DSB. During command buffer overflow, a warning  is thrown
> - * and rest all erroneous condition register programming is done
> - * through mmio write.
> - */
> -void intel_dsb_reg_write(struct intel_dsb *dsb,
> -			 i915_reg_t reg, u32 val)
> -{
> -	if (!assert_dsb_has_room(dsb))
> -		return;
> -
> -	intel_dsb_emit(dsb, val,
> -		       (DSB_OPCODE_MMIO_WRITE << DSB_OPCODE_SHIFT) |
> -		       (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
> -		       i915_mmio_reg_offset(reg));
>  }
> 
>  /**
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h
> b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 25f13c4d5389..25d774049cc2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -17,8 +17,6 @@ struct intel_dsb *intel_dsb_prepare(struct intel_crtc
> *crtc);  void intel_dsb_cleanup(struct intel_dsb *dsb);  void
> intel_dsb_reg_write(struct intel_dsb *dsb,
>  			 i915_reg_t reg, u32 val);
> -void intel_dsb_indexed_reg_write(struct intel_dsb *dsb,
> -				 i915_reg_t reg, u32 val);
>  void intel_dsb_commit(struct intel_dsb *dsb);
> 
>  #endif
> --
> 2.37.4





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux