Re: [PATCH v3 01/15] drm/msm/dpu: add formats check for writeback encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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






[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux