Re: [PATCH v6 4/7] drm: writeback: Modify drm_writeback_signal_completion helper

On 7/13/26 22:28, Suraj Kandpal wrote:
> drm_writeback_signal_completion() needs access to the parent
> drm_connector object as well as the drm_writeback_connector object
> itself. So, pass in the top level drm_connector and traverse down
> to drm_writeback_connector rather than passing in the lower level
> object and traversing back up. Update to use the top level object
> for consistency across the writeback interface.
> 
> Signed-off-by: Suraj Kandpal <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Reviewed-by: John Harrison <[email protected]>
> ---
> v5 -> v6:
> - Rebase over latest kernel
> 
> v4 -> v5:
> - Make @connector kerneldoc wording consistent across the series (John)
> 
> v3 -> v4:
> - Update subject line for consitency (John)
> - Update commit message across commits for consitency (John)
> 
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 2 +-
>   drivers/gpu/drm/arm/display/komeda/komeda_crtc.c    | 2 +-
>   drivers/gpu/drm/arm/malidp_hw.c                     | 6 +++---
>   drivers/gpu/drm/drm_writeback.c                     | 6 ++++--
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 ++--
>   drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c | 2 +-
>   drivers/gpu/drm/vc4/vc4_txp.c                       | 2 +-
>   drivers/gpu/drm/vkms/vkms_composer.c                | 2 +-
>   include/drm/drm_writeback.h                         | 2 +-
>   9 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 9504f6b5571e..6c507eeeda84 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4529,7 +4529,7 @@ bool amdgpu_dm_crtc_complete_writeback(struct amdgpu_crtc *acrtc)
>   	if (!pending)
>   		return false;
>   
> -	drm_writeback_signal_completion(acrtc->wb_conn, 0);
> +	drm_writeback_signal_completion(acrtc->connector, 0);
The change broke IGT's kms_writeback (null pointer deferences).

It can be fixed by "drm_writeback_to_connector", i.e., 
drm_writeback_signal_completion(drm_writeback_to_connector(acrtc->wb_conn), 
0);

>   	drm_crtc_vblank_put(&acrtc->base);
>   
>   	return true;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index c64cf5d97e62..da6bfe2797aa 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -213,7 +213,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>   		struct komeda_wb_connector *wb_conn = kcrtc->wb_conn;
>   
>   		if (wb_conn)
> -			drm_writeback_signal_completion(&wb_conn->base.writeback, 0);
> +			drm_writeback_signal_completion(&wb_conn->base, 0);
>   		else
>   			drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n",
>   				 drm_crtc_index(&kcrtc->base));
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index 5a7bd27d3718..9b845d3f34e1 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -1315,15 +1315,15 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
>   	if (status & se->vsync_irq) {
>   		switch (hwdev->mw_state) {
>   		case MW_ONESHOT:
> -			drm_writeback_signal_completion(&malidp->mw_connector.writeback, 0);
> +			drm_writeback_signal_completion(&malidp->mw_connector, 0);
>   			break;
>   		case MW_STOP:
> -			drm_writeback_signal_completion(&malidp->mw_connector.writeback, 0);
> +			drm_writeback_signal_completion(&malidp->mw_connector, 0);
>   			/* disable writeback after stop */
>   			hwdev->mw_state = MW_NOT_ENABLED;
>   			break;
>   		case MW_RESTART:
> -			drm_writeback_signal_completion(&malidp->mw_connector.writeback, 0);
> +			drm_writeback_signal_completion(&malidp->mw_connector, 0);
>   			fallthrough;	/* to a new start */
>   		case MW_START:
>   			/* writeback started, need to emulate one-shot mode */
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index e999eca5b649..df8484a7aa03 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -480,7 +480,8 @@ static void cleanup_work(struct work_struct *work)
>   
>   /**
>    * drm_writeback_signal_completion - Signal the completion of a writeback job
> - * @wb_connector: The writeback connector whose job is complete
> + * @connector: DRM connector which contains the writeback connector whose
> + * job is complete
>    * @status: Status code to set in the writeback out_fence (0 for success)
>    *
>    * Drivers should call this to signal the completion of a previously queued
> @@ -495,10 +496,11 @@ static void cleanup_work(struct work_struct *work)
>    * See also: drm_writeback_queue_job()
>    */
>   void
> -drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
> +drm_writeback_signal_completion(struct drm_connector *connector,
>   				int status)
>   {
>   	unsigned long flags;
> +	struct drm_writeback_connector *wb_connector = &connector->writeback;
>   	struct drm_writeback_job *job;
>   	struct dma_fence *out_fence;
>   
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> index 0a4026f22274..977fc0337fbd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> @@ -370,7 +370,7 @@ static void dpu_encoder_phys_wb_done_irq(void *arg)
>   	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
>   
>   	if (wb_enc->wb_conn)
> -		drm_writeback_signal_completion(wb_enc->wb_conn, 0);
> +		drm_writeback_signal_completion(drm_writeback_to_connector(wb_enc->wb_conn), 0);
>   
>   	/* Signal any waiting atomic commit thread */
>   	wake_up_all(&phys_enc->pending_kickoff_wq);
> @@ -431,7 +431,7 @@ static void _dpu_encoder_phys_wb_handle_wbdone_timeout(
>   	phys_enc->enable_state = DPU_ENC_ERR_NEEDS_HW_RESET;
>   
>   	if (wb_enc->wb_conn)
> -		drm_writeback_signal_completion(wb_enc->wb_conn, 0);
> +		drm_writeback_signal_completion(drm_writeback_to_connector(wb_enc->wb_conn), 0);
>   
>   	dpu_encoder_frame_done_callback(phys_enc->parent, phys_enc, frame_event);
>   }
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> index 5cd6c81a9710..4b0f6cd46acb 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
> @@ -251,5 +251,5 @@ void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,
>   
>   void rcar_du_writeback_complete(struct rcar_du_crtc *rcrtc)
>   {
> -	drm_writeback_signal_completion(&rcrtc->wb_connector.writeback, 0);
> +	drm_writeback_signal_completion(&rcrtc->wb_connector, 0);
>   }
> diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
> index 9cf2ec99bdfb..4c756702b57e 100644
> --- a/drivers/gpu/drm/vc4/vc4_txp.c
> +++ b/drivers/gpu/drm/vc4/vc4_txp.c
> @@ -504,7 +504,7 @@ static irqreturn_t vc4_txp_interrupt(int irq, void *data)
>   	 */
>   	TXP_WRITE(TXP_DST_CTRL, TXP_READ(TXP_DST_CTRL) & ~TXP_EI);
>   	vc4_crtc_handle_vblank(vc4_crtc);
> -	drm_writeback_signal_completion(&txp->connector.writeback, 0);
> +	drm_writeback_signal_completion(&txp->connector, 0);
>   
>   	return IRQ_HANDLED;
>   }
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index 27fb6a7b55bb..83d217085ad0 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -652,7 +652,7 @@ void vkms_composer_worker(struct work_struct *work)
>   		return;
>   
>   	if (wb_pending) {
> -		drm_writeback_signal_completion(&out->wb_connector.writeback, 0);
> +		drm_writeback_signal_completion(&out->wb_connector, 0);
>   		spin_lock_irq(&out->composer_lock);
>   		crtc_state->wb_pending = false;
>   		spin_unlock_irq(&out->composer_lock);
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index b4c11d380df0..5e8ab51c2da4 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -100,7 +100,7 @@ void drm_writeback_queue_job(struct drm_connector *wb_connector,
>   void drm_writeback_cleanup_job(struct drm_writeback_job *job);
>   
>   void
> -drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
> +drm_writeback_signal_completion(struct drm_connector *connector,
>   				int status);
>   
>   struct dma_fence *