> -----Original Message----- > From: Intel-gfx <[email protected]> On Behalf Of Ville > Syrjala > Sent: Friday, December 16, 2022 6:08 AM > To: [email protected] > Subject: [Intel-gfx] [PATCH 10/13] drm/i915/dsb: Allow the caller to pass in > the DSB buffer size > > From: Ville Syrjälä <[email protected]> > > 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ä <[email protected]>
LGTM. Reviewed-by: Animesh Manna <[email protected]> > --- > 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
