Re: [PATCH v3 17/26] drm/i915/writeback: Add the enable sequence from writeback

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

On 3/25/2026 4:37 PM, Suraj Kandpal wrote:
> Add enable sequence for writeback, use encoder->enable hook to
> enable the transcoder.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   .../gpu/drm/i915/display/intel_writeback.c    | 103 ++++++++++++++++++
>   1 file changed, 103 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_writeback.c b/drivers/gpu/drm/i915/display/intel_writeback.c
> index ba4c162847c8..d45d5faaf7cc 100644
> --- a/drivers/gpu/drm/i915/display/intel_writeback.c
> +++ b/drivers/gpu/drm/i915/display/intel_writeback.c
> @@ -17,8 +17,10 @@
>   
>   #include "intel_atomic.h"
>   #include "intel_connector.h"
> +#include "intel_crtc.h"
>   #include "intel_de.h"
>   #include "intel_display_driver.h"
> +#include "intel_display_regs.h"
>   #include "intel_display_types.h"
>   #include "intel_display_utils.h"
>   #include "intel_fb_pin.h"
> @@ -30,6 +32,7 @@ struct intel_writeback_connector {
>   	struct intel_encoder encoder;
>   	struct intel_writeback_job *job;
>   	enum transcoder trans;
> +	enum pipe pipe;
>   	int frame_num;
>   };
>   
> @@ -48,6 +51,12 @@ static const u32 writeback_formats[] = {
>   	DRM_FORMAT_XBGR2101010,
>   };
>   
> +static struct intel_writeback_connector
> +*enc_to_intel_writeback_connector(struct intel_encoder *encoder)
> +{
> +	return container_of(encoder, struct intel_writeback_connector, encoder);
> +}
> +
>   static int intel_writeback_connector_init(struct intel_connector *connector)
>   {
>   	struct intel_digital_connector_state *conn_state;
> @@ -215,6 +224,99 @@ static int intel_writeback_atomic_check(struct drm_connector *connector,
>   	return 0;
>   }
>   
> +static void intel_writeback_enable_encoder(struct intel_atomic_state *state,
> +					   struct intel_encoder *encoder,
> +					   const struct intel_crtc_state *crtc_state,
> +					   const struct drm_connector_state *conn_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct intel_writeback_connector *wb_conn =
> +		enc_to_intel_writeback_connector(encoder);
> +	struct intel_writeback_job *job = wb_conn->job;
> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +	enum transcoder trans = crtc_state->cpu_transcoder;
> +	struct intel_crtc *pipe_crtc;
> +	struct drm_framebuffer *fb;
> +	u32 val = 0, hactive, vactive;
> +	int i = 0;
> +
> +	if (!conn_state->writeback_job)
> +		return;
> +
> +	wb_conn->trans = trans;
> +	wb_conn->pipe = crtc->pipe;
> +	fb = job->fb;
> +	hactive = adjusted_mode->hdisplay;
> +	vactive = adjusted_mode->vdisplay;
> +
> +	/* Configure WD_STRIDE, WD_SURF and WD_TAIL_CFG */
> +	/* Enable Planes, Pipes and Transcoder */
> +	/* TRANSCODER TIMINGS and other transcoder setting*/
> +	/* minimum hactive as per bspec: 64 pixels */
> +	if (hactive < 64)
> +		drm_err(display->drm, "hactive is less then 64 pixels\n");
> +
> +	intel_de_write(display, TRANS_HTOTAL(display, trans), HACTIVE(hactive - 1));
> +	intel_de_write(display, TRANS_VTOTAL(display, trans), VACTIVE(vactive - 1));
> +
> +	val = 0;
> +	/* 2f) Configure and enable TRANS_WD_FUNC_CTL */
> +	switch (crtc->pipe) {
> +	default:
> +		fallthrough;
> +	case PIPE_A:
> +		val |= WD_INPUT_PIPE_A;
> +		break;
> +	case PIPE_B:
> +		val |= WD_INPUT_PIPE_B;
> +		break;
> +	case PIPE_C:
> +		val |= WD_INPUT_PIPE_C;
> +		break;
> +	case PIPE_D:
> +		val |= WD_INPUT_PIPE_D;
> +		break;
> +	}
> +
> +	switch (fb->format->format) {
> +	default:
> +		fallthrough;
> +	case DRM_FORMAT_YUYV:
> +		val |= WD_PIX_FMT_YUYV;
> +		break;
> +	case DRM_FORMAT_XYUV8888:
> +		val |= WD_PIX_FMT_XYUV8888;
> +		break;
> +	case DRM_FORMAT_XBGR8888:
> +		val |= WD_PIX_FMT_XBGR8888;
> +		break;
> +	case DRM_FORMAT_XBGR2101010:
> +		val |= WD_PIX_FMT_XBGR2101010;
> +		break;
> +	}

Both these can be moved to their own get() functions. Also please add
MISSING_CASE().

> +
> +	val |= TRANS_WD_FUNC_ENABLE | WD_TRIGGERED_CAP_MODE_ENABLE |
> +		WD_DISABLE_POINTERS;
> +	intel_de_write(display, WD_TRANS_FUNC_CTL(trans), val);
> +
> +	if (DISPLAY_VER(display) >= 13)
> +		intel_de_rmw(display, PIPE_CHICKEN(crtc->pipe),
> +			     UNDERRUN_RECOVERY_DISABLE_ADLP,
> +			     UNDERRUN_RECOVERY_DISABLE_ADLP);
> +
> +	/*  Configure and enable TRANS_CONF */
> +	intel_de_write(display, TRANSCONF_WD(trans), WD_TRANS_ENABLE);
> +	intel_de_posting_read(display, TRANSCONF_WD(trans));
> +
> +	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, crtc_state, i) {
> +		const struct intel_crtc_state *pipe_crtc_state =
> +			intel_atomic_get_new_crtc_state(state, pipe_crtc);
> +
> +		intel_crtc_vblank_on(pipe_crtc_state);
> +	}
> +}
> +
>   static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
>   	.destroy = drm_encoder_cleanup,
>   };
> @@ -335,6 +437,7 @@ int intel_writeback_init(struct intel_display *display)
>   	encoder->get_config = intel_writeback_get_config;
>   	encoder->get_hw_state = intel_writeback_get_hw_state;
>   	encoder->compute_config = intel_writeback_compute_config;
> +	encoder->enable = intel_writeback_enable_encoder;
>   
>   	connector = &writeback_conn->connector;
>   	ret = intel_writeback_connector_alloc(connector);