Commit f2969c49 authored by Abhinav Kumar's avatar Abhinav Kumar Committed by Dmitry Baryshkov
Browse files

drm/msm/dpu: gracefully handle null fb commits for writeback 



kms_writeback test cases also verify with a null fb for the
writeback connector job. In addition there are also other
commit paths which can result in kickoffs without a valid
framebuffer like while closing the fb which results in the
callback to drm_atomic_helper_dirtyfb() which internally
triggers a commit.

Add protection in the dpu driver to ensure that commits for
writeback encoders without a valid fb are gracefully skipped.

changes in v2:
	- rename dpu_encoder_has_valid_fb to dpu_encoder_is_valid_for_commit

changes in v3:
	- none

Signed-off-by: default avatarAbhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: default avatarDmitry Baryshkov <dmitry.baryshkov@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/483522/
Link: https://lore.kernel.org/r/1650984096-9964-17-git-send-email-quic_abhinavk@quicinc.com


Signed-off-by: default avatarDmitry Baryshkov <dmitry.baryshkov@linaro.org>
parent 5cf5afcd

drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

+9 −0
Original line number Original line Diff line number Diff line
@@ -869,6 +869,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) 




	DPU_ATRACE_BEGIN("crtc_commit");

	DPU_ATRACE_BEGIN("crtc_commit");





	drm_for_each_encoder_mask(encoder, crtc->dev,

			crtc->state->encoder_mask) {

		if (!dpu_encoder_is_valid_for_commit(encoder)) {

			DRM_DEBUG_ATOMIC("invalid FB not kicking off crtc\n");

			goto end;

		}

	}

	/*

	/*

	 * Encoder will flush/start now, unless it has a tx pending. If so, it

	 * Encoder will flush/start now, unless it has a tx pending. If so, it

	 * may delay and flush at an irq event (e.g. ppdone)

	 * may delay and flush at an irq event (e.g. ppdone)
@@ -891,6 +898,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) 
		dpu_encoder_kickoff(encoder);

		dpu_encoder_kickoff(encoder);





	reinit_completion(&dpu_crtc->frame_done_comp);

	reinit_completion(&dpu_crtc->frame_done_comp);



end:

	DPU_ATRACE_END("crtc_commit");

	DPU_ATRACE_END("crtc_commit");

}

}

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

+21 −0
Original line number Original line Diff line number Diff line
@@ -1862,6 +1862,27 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) 
		dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc);

		dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc);

}

}





bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc)

{

	struct dpu_encoder_virt *dpu_enc;

	unsigned int i;

	struct dpu_encoder_phys *phys;



	dpu_enc = to_dpu_encoder_virt(drm_enc);



	if (drm_enc->encoder_type == DRM_MODE_ENCODER_VIRTUAL) {

		for (i = 0; i < dpu_enc->num_phys_encs; i++) {

			phys = dpu_enc->phys_encs[i];

			if (phys->ops.is_valid_for_commit && !phys->ops.is_valid_for_commit(phys)) {

				DPU_DEBUG("invalid FB not kicking off\n");

				return false;

			}

		}

	}



	return true;

}



void dpu_encoder_kickoff(struct drm_encoder *drm_enc)

void dpu_encoder_kickoff(struct drm_encoder *drm_enc)

{

{

	struct dpu_encoder_virt *dpu_enc;

	struct dpu_encoder_virt *dpu_enc;

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h

+6 −0
Original line number Original line Diff line number Diff line
@@ -196,4 +196,10 @@ void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc, 
void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,

void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,

		struct drm_writeback_job *job);

		struct drm_writeback_job *job);





/**

 * dpu_encoder_is_valid_for_commit - check if encode has valid parameters for commit.

 * @drm_enc:    Pointer to drm encoder structure

 */

bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc);



#endif /* __DPU_ENCODER_H__ */
 
#endif /* __DPU_ENCODER_H__ */

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h

+1 −0
Original line number Original line Diff line number Diff line
@@ -142,6 +142,7 @@ struct dpu_encoder_phys_ops { 
			struct drm_writeback_job *job);

			struct drm_writeback_job *job);

	void (*cleanup_wb_job)(struct dpu_encoder_phys *phys_enc,

	void (*cleanup_wb_job)(struct dpu_encoder_phys *phys_enc,

			struct drm_writeback_job *job);

			struct drm_writeback_job *job);

	bool (*is_valid_for_commit)(struct dpu_encoder_phys *phys_enc);

};

};





/**

/**

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c

+12 −0
Original line number Original line Diff line number Diff line
@@ -667,6 +667,16 @@ static void dpu_encoder_phys_wb_cleanup_wb_job(struct dpu_encoder_phys *phys_enc 
	wb_enc->wb_conn = NULL;

	wb_enc->wb_conn = NULL;

}

}





static bool dpu_encoder_phys_wb_is_valid_for_commit(struct dpu_encoder_phys *phys_enc)

{

	struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc);



	if (wb_enc->wb_job)

		return true;

	else

		return false;

}



/**

/**

 * dpu_encoder_phys_wb_init_ops - initialize writeback operations

 * dpu_encoder_phys_wb_init_ops - initialize writeback operations

 * @ops:	Pointer to encoder operation table

 * @ops:	Pointer to encoder operation table
@@ -687,6 +697,8 @@ static void dpu_encoder_phys_wb_init_ops(struct dpu_encoder_phys_ops *ops) 
	ops->prepare_wb_job = dpu_encoder_phys_wb_prepare_wb_job;

	ops->prepare_wb_job = dpu_encoder_phys_wb_prepare_wb_job;

	ops->cleanup_wb_job = dpu_encoder_phys_wb_cleanup_wb_job;

	ops->cleanup_wb_job = dpu_encoder_phys_wb_cleanup_wb_job;

	ops->irq_control = dpu_encoder_phys_wb_irq_ctrl;

	ops->irq_control = dpu_encoder_phys_wb_irq_ctrl;

	ops->is_valid_for_commit = dpu_encoder_phys_wb_is_valid_for_commit;



}

}





/**

/**