On 12/11/2023 10:40 PM, Dmitry Baryshkov wrote:
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:In preparation for adding more formats to dpu writeback add format validation to it to fail any unsupported formats. changes in v3: - rebase on top of msm-next - replace drm_atomic_helper_check_wb_encoder_state() with drm_atomic_helper_check_wb_connector_state() due to the rebase changes in v2: - correct some grammar in the commit text Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback") Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++++++ 1 file changed, 7 insertions(+) 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 bb94909caa25..425415d45ec1 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 @@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check( { struct drm_framebuffer *fb; const struct drm_display_mode *mode = &crtc_state->mode; + int ret; DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n", phys_enc->hw_wb->idx, mode->name, mode->hdisplay, mode->vdisplay); @@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check( return -EINVAL; } + ret = drm_atomic_helper_check_wb_connector_state(conn_state->connector, conn_state->state); + if (ret < 0) { + DPU_ERROR("invalid pixel format %p4cc\n", &fb->format->format); + return ret; + }There is no guarantee that there will be no other checks added to this helper. So, I think this message is incorrect. If you wish, you can promote the level of the message in the helper itself. On the other hand, we rarely print such messages by default. Most of the checks use drm_dbg.
hmm...actually drm_atomic_helper_check_wb_connector_state() already has a debug message to indicate invalid pixel formats.
You are right, i should perhaps just say that "atomic_check failed" or something.
I can make this a DPU_DEBUG. Actually I didnt know that we are not supposed to print out atomic_check() errors. Is it perhaps because its okay for check to fail?
But then we would not know why it failed.
+ return 0; } -- 2.40.1
