Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/fourcc: documented UBWC P010 layout mismatches msm driver --
commit 9d1d25812fbfd6458cdaf0aaead9ba0fe219bc18 Author: Dmitry Baryshkov <[email protected]> drm/fourcc: describe the Qualcomm UBWC layout This commit documents the buffer layout, plane ordering, tile sizes, and alignment requirements for DRM_FORMAT_MOD_QCOM_COMPRESSED across RGB, DRM_FORMAT_NV12, and DRM_FORMAT_P010 formats. > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index 0bb049f91524e..4b6a4a827f6b8 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -820,13 +820,53 @@ extern "C" { [ ... ] > + * DRM_FORMAT_P010, two planes, Y then CbCr, 16 bits per sample: > + * Y data: pitch ALIGN(width * 2, 256), ALIGN(height, 16) lines > + * Y metadata: 32x4 pixel tiles, stride ALIGN(DIV_ROUND_UP(width, 32), 64), > + * ALIGN(DIV_ROUND_UP(height, 4), 16) lines > + * CbCr data: pitch ALIGN(width * 2, 256), > + * ALIGN(DIV_ROUND_UP(height, 2), 16) lines > + * CbCr metadata: 16x4 sample tiles, > + * stride ALIGN(DIV_ROUND_UP(DIV_ROUND_UP(width, 2), 16), 64), > + * ALIGN(DIV_ROUND_UP(DIV_ROUND_UP(height, 2), 4), 16) lines [Severity: Medium] Does the documented layout for DRM_FORMAT_P010 match what the msm driver currently expects for DRM_FORMAT_MOD_QCOM_COMPRESSED? In drivers/gpu/drm/msm/disp/mdp_format.c, mdp_formats_ubwc[] defines the compressed P010 entry using PSEUDO_YUV_FMT_TILED, which unconditionally sets MSM_FORMAT_FLAG_UNPACK_TIGHT: drivers/gpu/drm/msm/disp/mdp_format.c: #define PSEUDO_YUV_FMT_TILED(fmt, a, r, g, b, e0, e1, chroma, flg, tile_height) \ ... .fetch_mode = MDP_FETCH_UBWC, \ .flags = MSM_FORMAT_FLAG_UNPACK_TIGHT | \ MSM_FORMAT_FLAG_YUV | \ MSM_FORMAT_FLAG_COMPRESSED | flg, \ static const struct msm_format mdp_formats_ubwc[] = { ... PSEUDO_YUV_FMT_TILED(P010, BPC8, BPC8, BPC8, C1_B_Cb, C2_R_Cr, CHROMA_420, MSM_FORMAT_FLAG_DX, MDP_TILE_HEIGHT_UBWC), When DRM_IOCTL_MODE_ADDFB2 calls msm_framebuffer_create() -> msm_framebuffer_init() -> mdp_format_ubwc_plane_sizes(), the presence of MSM_FORMAT_FLAG_UNPACK_TIGHT causes it to take the tightly packed TP10/QC10C branch instead of the 16-bit per sample else branch described in this comment: drivers/gpu/drm/msm/disp/mdp_format.c:mdp_format_ubwc_plane_sizes() { ... if (MSM_FORMAT_IS_DX(fmt)) { if (fmt->flags & MSM_FORMAT_FLAG_UNPACK_TIGHT) { /* can't use round_up() here because 192 is NPoT */ stride = roundup(width, 192); stride = round_up(stride * 4 / 3, 256); y_tile_width = 48; } else { stride = round_up(width * 2, 256); y_tile_width = 32; } ... } Because msm_framebuffer_init() checks that mode_cmd->pitches[0] equals ubwc_pitch[0]: drivers/gpu/drm/msm/msm_fb.c:msm_framebuffer_init() { ... } else if (mode_cmd->pitches[0] != ubwc_pitch[0]) { ret = UERR(EINVAL, dev, "pitch %u, expected %u", mode_cmd->pitches[0], ubwc_pitch[0]); goto fail; } ... } won't a DRM_FORMAT_P010 framebuffer allocated with DRM_FORMAT_MOD_QCOM_COMPRESSED according to this documentation (with pitch ALIGN(width * 2, 256) and 32x4 Y tiles) be rejected with -EINVAL by msm_framebuffer_init(), or have the DPU unpacker configured with UNPACK_TIGHT = 1 and UNPACK_ALIGN_MSB = 0 in dpu_hw_setup_format_impl() and dpu_hw_wb_setup_format()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
