Re: [PATCH v3 21/26] drm/i915/writeback: Configure WD_SURF register

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 3/25/2026 4:37 PM, Suraj Kandpal wrote:
> Get the ggtt_offset of the drm_framebuffer which needs to be
> written to the surface base address bits of WD_SURF register.
> 

Why not do all the setup work first and add both WD_SURF/WD_STRIDE 
programming along with the introduction of intel_writeback_capture()?

> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_writeback.c     | 4 ++++
>   drivers/gpu/drm/i915/display/intel_writeback_reg.h | 2 ++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_writeback.c b/drivers/gpu/drm/i915/display/intel_writeback.c
> index e2f7c46015d2..54e74450e080 100644
> --- a/drivers/gpu/drm/i915/display/intel_writeback.c
> +++ b/drivers/gpu/drm/i915/display/intel_writeback.c
> @@ -267,6 +267,7 @@ static void intel_writeback_capture(struct intel_atomic_state *state,
>   	const struct drm_display_mode *adjusted_mode =
>   		&crtc_state->hw.adjusted_mode;
>   	struct drm_writeback_job *wb_job = conn_state->writeback_job;
> +	struct intel_writeback_job *job = conn_state->writeback_job->priv;
>   	enum transcoder trans = wb_conn->trans;
>   	u32 val = 0;
>   	int bpp;
> @@ -275,6 +276,9 @@ static void intel_writeback_capture(struct intel_atomic_state *state,
>   	val = DIV_ROUND_UP((adjusted_mode->hdisplay * bpp), 64);
>   	intel_de_write(display, WD_STRIDE(trans), WD_STRIDE_VAL(val));
>   
> +	val = intel_get_ggtt_addr(job->vma);
> +	intel_de_write(display, WD_SURF(trans), val);
> +
>   	val = 0;
>   	val |= START_TRIGGER_FRAME | WD_FRAME_NUMBER(wb_conn->frame_num);
>   	intel_de_rmw(display, WD_TRANS_FUNC_CTL(trans),
> diff --git a/drivers/gpu/drm/i915/display/intel_writeback_reg.h b/drivers/gpu/drm/i915/display/intel_writeback_reg.h
> index f526af0f9aff..403f9b64015b 100644
> --- a/drivers/gpu/drm/i915/display/intel_writeback_reg.h
> +++ b/drivers/gpu/drm/i915/display/intel_writeback_reg.h
> @@ -81,6 +81,8 @@
>   #define WD_SURF(tc)			_MMIO_WD(tc,\
>   					_WD_SURF_0,\
>   					_WD_SURF_1)
> +#define  WD_SURF_ADDR_MASK		REG_GENMASK(31, 12)
> +#define  WD_SURF_ADDR(val)		REG_FIELD_PREP(WD_SURF_ADDR_MASK, val)

Unused and perhaps not needed. I guess direct assignment of val works 
because addresses need to be 4KB aligned.

>   
>   #define _WD_IMR_0			0x6e560
>   #define _WD_IMR_1			0x6ed60