Messages in this thread | | | Date | Tue, 12 Dec 2023 09:16:59 -0800 | Subject | Re: [PATCH v3 01/15] drm/msm/dpu: add formats check for writeback encoder | From | Abhinav Kumar <> |
| |
On 12/11/2023 10:40 PM, Dmitry Baryshkov wrote: > On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar <quic_abhinavk@quicinc.com> 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@quicinc.com> >> --- >> 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 >> > >
| |