Re: [PATCH i-g-t v1 0/3] Revert HDCP per crtc infrastructure

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

On Thu, 2026-05-14 at 15:05 +0530, Santhosh Reddy Guddati wrote:
> Reverts below commits
> 
> b4036b1a0 tests/kms_content_protection: Add FB cleanup for MST tests
> 8d7c2453d tests/kms_content_protection: Pass crtc parameter and use
> per-CRTC FBs
> 925b14d43 tests/kms_content_protection: Add per-CRTC framebuffer
> infrastructure
> 
> The per-CRTC fb introduced in the commits caused regression
> especially
> when the eDP panel is connected with 2 4k Panels connected through a
> DP-MST hub with "-ENOSPC" and all the content protection tests are
> failing.
> 
> The MST subtests are skipping when subtests are executed with "--r
> dp-mst*" , as the MST changes introduced did not clean up properly
> across subtests.
> 
> The create_fbs() now iterated through all crtcs and eDP (2k) claims
> the first
> crtc before the larger output and fb is created with smaller size.
> Later when DP-4 maps to
> the smae CRTC (it has differnt fb dimensions now 2k fb than the mode
> required) , the kernel rejects the commits  with ENOSPC because of
> invalid coordinates.
> 

Hi Santhosh,

It seems you and I have a conflicting issue:
[1] Undersized buffers: When 4K output uses CRTC with 2K FB
[2] Oversized buffers: When 3504x2190 panel would get 3840x2160 FB

[1] that requires reverting to maximum resolution, which in turn causes
my test scenario [2] to regress.

Therefore, I think we should find a solution that satisfies both needs,
rather than simply reverting my entire series.


Here is my solution:
Dynamically check and recreate framebuffers in modeset_with_fb():
- If existing FB is large enough → reuse it
- If existing FB is too small → remove and recreate with required size
- Only allocate what's actually needed, when needed

This provides optimal memory usage while preventing both undersized and
oversized buffer issues.


Here is a simple patch for this:

+static void ensure_fb_size(igt_crtc_t *crtc, int width, int height)
+{
+       struct hdcp_test_fbs *fbs = &data.fbs[crtc->crtc_index];
+
+       /*
+        * Check if existing framebuffers are large enough.
+        * Recreate only if they are too small for the current mode.
+        */
+       if (fbs->red.fb_id &&
+           fbs->red.width >= width && fbs->red.height >= height)
+               return;
+
+       /* Remove existing framebuffers if they exist */
+       if (fbs->red.fb_id) {
+               igt_remove_fb(data.drm_fd, &fbs->red);
+               igt_remove_fb(data.drm_fd, &fbs->green);
+       }
+
+       /* Create new framebuffers with the required size */
+       igt_create_color_fb(data.drm_fd, width, height,
+                           DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
+                           1.f, 0.f, 0.f, &fbs->red);
+       igt_create_color_fb(data.drm_fd, width, height,
+                           DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
+                           0.f, 1.f, 0.f, &fbs->green);
+}
 static void modeset_with_fb(igt_output_t *output, igt_crtc_t *crtc,
                             enum igt_commit_style commit_style)
 {
         igt_display_t *display = &data.display;
         drmModeModeInfo *mode;
         igt_plane_t *primary;

         mode = igt_output_get_mode(output);

+       /* Ensure framebuffers are large enough for this mode */
+       ensure_fb_size(crtc, mode->hdisplay, mode->vdisplay);
+
        primary = igt_output_get_plane_type(output,
DRM_PLANE_TYPE_PRIMARY);
        igt_plane_set_fb(primary, &data.fbs[crtc->crtc_index].red);
        igt_fb_set_size(&data.fbs[crtc->crtc_index].red, primary, mode-
>hdisplay, mode->vdisplay);
@@ -1051,27 +1077,27 @@ static void
test_content_protection_cleanup(void)
 static void create_fbs(void)
  {
              drmModeModeInfo *mode;
              igt_output_t *output;
              igt_crtc_t *crtc;

-       /* Create framebuffers for each connected output's pipe */
+       /*
+        * Create initial framebuffers for each CRTC that has a valid
output.
+        * These may be recreated later if a larger mode is needed.
+        * This approach avoids creating oversized buffers
unnecessarily while
+        * allowing dynamic resizing when needed.
+        */
        for_each_connected_output(&data.display, output) {
                mode = igt_output_get_mode(output);
                igt_assert(mode);

-               /* Find a valid crtc for this output */
                for_each_crtc(&data.display, crtc) {
                        if (!igt_crtc_connector_valid(crtc, output))
                                continue;

-                       /* Skip if already created for this crtc */
+                       /* Create FB for this CRTC if not already done
*/
                        if (data.fbs[crtc->crtc_index].red.fb_id)
                                continue;

-                       igt_create_color_fb(data.drm_fd, mode-
>hdisplay, mode->vdisplay,
-                                           DRM_FORMAT_XRGB8888,
DRM_FORMAT_MOD_LINEAR,
-                                           1.f, 0.f, 0.f,
&data.fbs[crtc->crtc_index].red);
-                       igt_create_color_fb(data.drm_fd, mode-
>hdisplay, mode->vdisplay,
-                                           DRM_FORMAT_XRGB8888,
DRM_FORMAT_MOD_LINEAR,
-                                           0.f, 1.f, 0.f,
&data.fbs[crtc->crtc_index].green);
+                       ensure_fb_size(crtc, mode->hdisplay, mode-
>vdisplay);
                        break;
                }
        }


Could you please give it a try? Thank you very much!

Regards,
Jason-JH Lin

> Santhosh Reddy Guddati (3):
>   Revert "tests/kms_content_protection: Add per-CRTC framebuffer
>     infrastructure"
>   Revert "tests/kms_content_protection: Pass crtc parameter and use
>     per-CRTC FBs"
>   Revert "tests/kms_content_protection: Add FB cleanup for MST tests"
> 
>  tests/kms_content_protection.c | 168 +++++++++----------------------
> --
>  1 file changed, 47 insertions(+), 121 deletions(-)
>