Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm: broken P010 UBWC pitch check in msm_framebuffer_init() --
commit 692349e38091784388304159b902edd6edf3ad3a Author: Dmitry Baryshkov <[email protected]> drm/msm: check UBWC framebuffers against their layout This commit updates msm_framebuffer_init() to validate UBWC framebuffers against their computed UBWC plane pitches and sizes rather than checking them as linear buffers. > diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c > index 1e236416e15f0..dd24aa7a5ee80 100644 > --- a/drivers/gpu/drm/msm/msm_fb.c > +++ b/drivers/gpu/drm/msm/msm_fb.c [ ... ] > @@ -177,14 +179,41 @@ msm_framebuffer_init(struct drm_device *dev, const > struct drm_format_info *info, > goto fail; > } > > + if (format->fetch_mode == MDP_FETCH_UBWC) > + mdp_format_ubwc_plane_sizes(format, mode_cmd->width, > + mode_cmd->height, ubwc_pitch, > + ubwc_size); > + > for (i = 0; i < n; i++) { > unsigned int width = mode_cmd->width / (i ? info->hsub : 1); > unsigned int height = mode_cmd->height / (i ? info->vsub : 1); > - unsigned int min_size; > - > - min_size = (height - 1) * mode_cmd->pitches[i] > - + width * info->cpp[i] > - + mode_cmd->offsets[i]; > + u64 min_size; > + > + if (format->fetch_mode == MDP_FETCH_UBWC) { > + /* > + * The CbCr planes follow the Y data in the first > buffer, > + * offsets[1] and pitches[1] are not used. > + */ > + if (i > 0) { > + if (bos[i] != bos[0]) { > + ret = UERR(EINVAL, dev, > + "plane %d not in the first > buffer", i); > + goto fail; > + } > + min_size = 0; > + } 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; [Severity: High] Does this break creating DRM_FORMAT_P010 framebuffers with DRM_FORMAT_MOD_QCOM_COMPRESSED? In drivers/gpu/drm/msm/disp/mdp_format.c, mdp_formats_ubwc[] defines the P010 entry using PSEUDO_YUV_FMT_TILED, which sets MSM_FORMAT_FLAG_UNPACK_TIGHT: drivers/gpu/drm/msm/disp/mdp_format.c: 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 mdp_format_ubwc_plane_sizes() is called on a DX YUV format with MSM_FORMAT_FLAG_UNPACK_TIGHT set, it computes the tightly-packed TP10 stride (4/3 bytes per pixel) rather than the 2 bytes-per-pixel P010 stride: 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; } ... } However, before msm_framebuffer_init() is reached, drm_internal_framebuffer_create() -> framebuffer_check() in drivers/gpu/drm/drm_framebuffer.c checks r->pitches[0] against drm_format_info_min_pitch() for DRM_FORMAT_P010 (which has char_per_block[0] = 2 and block_w[0] = 1), rejecting any framebuffer where pitches[0] < width * 2: drivers/gpu/drm/drm_framebuffer.c:framebuffer_check() { ... if (block_size && r->pitches[i] < min_pitch) { drm_dbg_kms(dev, "bad pitch %u for plane %d\n", r->pitches[i], i); return -EINVAL; } ... } Because roundup(width, 192) * 4 / 3 is strictly less than width * 2 for all widths > 256 (as well as 129 <= width <= 192), no pitches[0] value can satisfy both framebuffer_check() (pitches[0] >= width * 2) and msm_framebuffer_init() (pitches[0] == ubwc_pitch[0]). For example, at width = 1920, framebuffer_check() requires pitches[0] >= 3840 while ubwc_pitch[0] is 2560, causing ADDFB2 to fail unconditionally with -EINVAL. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
