Re: [PATCH 10/13] drm/i915/dsb: Allow the caller to pass in the DSB buffer size

[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 10/13] drm/i915/dsb: Allow the caller to pass in
> the DSB buffer size
> 
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> The caller should more or less know how many DSB commands it wants to
> emit into the command buffer, so allow it to specify the size of the command
> buffer rather than having the low level DSB code guess it.
> 
> Technically we can emit as many as 134+1033 (for adl+ degamma + 10bit
> gamma) register writes but thanks to the DSB indexed register write
> command we get significant space savings so the current size estimate of
> 8KiB (~1024 DSB commands) is sufficient for now.
> 
> 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 |  2 +-
>  drivers/gpu/drm/i915/display/intel_dsb.c   | 42 +++++++++++++---------
>  drivers/gpu/drm/i915/display/intel_dsb.h   |  3 +-
>  3 files changed, 29 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 76603357252d..8d97c299e657 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1380,7 +1380,7 @@ void intel_color_prepare_commit(struct
> intel_crtc_state *crtc_state)
>  	/* FIXME DSB has issues loading LUTs, disable it for now */
>  	return;
> 
> -	crtc_state->dsb = intel_dsb_prepare(crtc);
> +	crtc_state->dsb = intel_dsb_prepare(crtc, 1024);
>  }
> 
>  void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state) diff --git
> a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 636c57767f97..7c593ec84d41 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -30,21 +30,24 @@ struct intel_dsb {
>  	struct intel_crtc *crtc;
> 
>  	/*
> -	 * free_pos will point the first free entry position
> -	 * and help in calculating tail of command buffer.
> +	 * maximum number of dwords the buffer will hold.
>  	 */
> -	int free_pos;
> +	unsigned int size;
> 
>  	/*
> -	 * ins_start_offset will help to store start address of the dsb
> +	 * free_pos will point the first free dword and
> +	 * help in calculating tail of command buffer.
> +	 */
> +	unsigned int free_pos;
> +
> +	/*
> +	 * ins_start_offset will help to store start dword of the dsb
>  	 * instuction and help in identifying the batch of auto-increment
>  	 * register.
>  	 */
> -	u32 ins_start_offset;
> +	unsigned int ins_start_offset;
>  };
> 
> -#define DSB_BUF_SIZE    (2 * PAGE_SIZE)
> -
>  /**
>   * DOC: DSB
>   *
> @@ -76,7 +79,7 @@ static bool assert_dsb_has_room(struct intel_dsb *dsb)
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> 
>  	/* each instruction is 2 dwords */
> -	return !drm_WARN(&i915->drm, ALIGN(dsb->free_pos, 2) >
> DSB_BUF_SIZE / 4 - 2,
> +	return !drm_WARN(&i915->drm, dsb->free_pos > dsb->size - 2,
>  			 "DSB buffer overflow\n");
>  }
> 
> @@ -168,7 +171,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb,
>  		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 + 0] = 1; /* count */
>  			buf[dsb->ins_start_offset + 1] =
>  				(DSB_OPCODE_INDEXED_WRITE <<
> DSB_OPCODE_SHIFT) |
>  				i915_mmio_reg_offset(reg);
> @@ -178,7 +181,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb,
>  		}
> 
>  		buf[dsb->free_pos++] = val;
> -		/* Update the size. */
> +		/* Update the count */
>  		buf[dsb->ins_start_offset]++;
> 
>  		/* if number of data words is odd, then the last dword
> should be 0.*/ @@ -250,6 +253,7 @@ void intel_dsb_commit(struct
> intel_dsb *dsb)
>  /**
>   * intel_dsb_prepare() - Allocate, pin and map the DSB command buffer.
>   * @crtc: the CRTC
> + * @max_cmds: number of commands we need to fit into command buffer
>   *
>   * This function prepare the command buffer which is used to store dsb
>   * instructions with data.
> @@ -257,25 +261,30 @@ void intel_dsb_commit(struct intel_dsb *dsb)
>   * Returns:
>   * DSB context, NULL on failure
>   */
> -struct intel_dsb *intel_dsb_prepare(struct intel_crtc *crtc)
> +struct intel_dsb *intel_dsb_prepare(struct intel_crtc *crtc,
> +				    unsigned int max_cmds)
>  {
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> -	struct intel_dsb *dsb;
>  	struct drm_i915_gem_object *obj;
> -	struct i915_vma *vma;
> -	u32 *buf;
>  	intel_wakeref_t wakeref;
> +	struct intel_dsb *dsb;
> +	struct i915_vma *vma;
> +	unsigned int size;
> +	u32 *buf;
> 
>  	if (!HAS_DSB(i915))
>  		return NULL;
> 
> -	dsb = kmalloc(sizeof(*dsb), GFP_KERNEL);
> +	dsb = kzalloc(sizeof(*dsb), GFP_KERNEL);
>  	if (!dsb)
>  		goto out;
> 
>  	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> 
> -	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
> +	/* ~1 qword per instruction, full cachelines */
> +	size = ALIGN(max_cmds * 8, CACHELINE_BYTES);
> +
> +	obj = i915_gem_object_create_internal(i915, PAGE_ALIGN(size));
>  	if (IS_ERR(obj))
>  		goto out_put_rpm;
> 
> @@ -297,6 +306,7 @@ struct intel_dsb *intel_dsb_prepare(struct intel_crtc
> *crtc)
>  	dsb->vma = vma;
>  	dsb->crtc = crtc;
>  	dsb->cmd_buf = buf;
> +	dsb->size = size / 4; /* in dwords */
>  	dsb->free_pos = 0;
>  	dsb->ins_start_offset = 0;
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h
> b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 25d774049cc2..05c221b6d0a4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -13,7 +13,8 @@
>  struct intel_crtc;
>  struct intel_dsb;
> 
> -struct intel_dsb *intel_dsb_prepare(struct intel_crtc *crtc);
> +struct intel_dsb *intel_dsb_prepare(struct intel_crtc *crtc,
> +				    unsigned int max_cmds);
>  void intel_dsb_cleanup(struct intel_dsb *dsb);  void
> intel_dsb_reg_write(struct intel_dsb *dsb,
>  			 i915_reg_t reg, u32 val);
> --
> 2.37.4





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

  Powered by Linux