Re: [PATCH v3 24/26] drm/i915/writeback: Define the disable sequence for writeback

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

On 3/25/2026 4:37 PM, Suraj Kandpal wrote:
> Define the disable sequence for a writeback encoder. We only disable
> the encoder if no writeback job is pending, if it is then we just
> need to disable the wd function so that values can be updated
> accordingly.

Where is this handled?

> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   .../gpu/drm/i915/display/intel_writeback.c    | 28 +++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_writeback.c b/drivers/gpu/drm/i915/display/intel_writeback.c
> index 864d4a28de10..870fbfdddef5 100644
> --- a/drivers/gpu/drm/i915/display/intel_writeback.c
> +++ b/drivers/gpu/drm/i915/display/intel_writeback.c
> @@ -559,6 +559,33 @@ void intel_writeback_isr_handler(struct intel_display *display)
>   	}
>   }
>   
> +static void
> +intel_writeback_disable_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(encoder);
> +	struct intel_writeback_connector *wb_conn =
> +		enc_to_intel_writeback_connector(encoder);
> +	struct intel_crtc *pipe_crtc;
> +	int i = 0;
> +
> +	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, crtc_state, i) {
> +		const struct intel_crtc_state *old_pipe_crtc_state =
> +			intel_atomic_get_old_crtc_state(state, pipe_crtc);
> +
> +		intel_crtc_vblank_off(old_pipe_crtc_state);
> +	}
> +
> +	intel_de_rmw(display, TRANSCONF_WD(crtc_state->cpu_transcoder), WD_TRANS_ENABLE,
> +		     REG_FIELD_PREP(WD_TRANS_ENABLE, 0));
> +	intel_de_rmw(display, WD_TRANS_FUNC_CTL(crtc_state->cpu_transcoder),
> +		     TRANS_WD_FUNC_ENABLE,
> +		     REG_FIELD_PREP(TRANS_WD_FUNC_ENABLE, 0));

All of the above appears to be steps that are traditionally handled in 
encoder->post_disable(). There is a FIXME in hsw_crtc_disable to 
collapse the hooks to one but until then it is probably a good idea to 
keep it uniform across encoders.

> +	wb_conn->frame_num = 1;
> +}
> +
>   int intel_writeback_init(struct intel_display *display)
>   {
>   	struct intel_encoder *encoder;
> @@ -587,6 +614,7 @@ int intel_writeback_init(struct intel_display *display)
>   	encoder->get_hw_state = intel_writeback_get_hw_state;
>   	encoder->compute_config = intel_writeback_compute_config;
>   	encoder->enable = intel_writeback_enable_encoder;
> +	encoder->disable = intel_writeback_disable_encoder;
>   
>   	connector = &writeback_conn->connector;
>   	ret = intel_writeback_connector_alloc(connector);