Re: [PATCH i-g-t 1/1] tests/kms_content_protection: Re-create framebuffer before each modeset

[Karthik B S <karthik.b.s@intel.com>]

Hi Jason-JH,

On 1/7/2026 11:59 AM, Jason-JH Lin (林睿祥) wrote:
> Hi Karthik,
>
> On Wed, 2026-01-07 at 10:34 +0530, Karthik B S wrote:
>> Hi,
>>
>> On 1/6/2026 3:51 PM, Jason-JH Lin wrote:
>>> The red and green framebuffers were previously created only once at
>>> the beginning of the test. If the test runs on multiple outputs
>>> with
>>> different resolutions, the pre-created framebuffer's size might not
>>> match the mode of a subsequent output, causing the modeset to fail.
>> I see that the test is already creating fb's considering the max
>> width/height among of all the available outputs, so ideally this
>> issue
>> should not be hit. Are we seeing any actual failures which this patch
>> fixes or just doing this as a fail safe?
> Yes, I did hit the issue.
> Our first output has a resolution 3504x2190, while the second output,
> which supports HDCP, is 3840x2160, so the framebuffer is set to
> 3840x2190.
>
> Since out platform doesn't support scaling, atomic_check() returns (-
> ERANGE) when the framebuffer height needs to be scaled from 2190 to
> 2160, and the test fails.
Ideally it should be cropping and not invoke scaling, but yea as this 
test is not about cropping/scaling its better to simplify this test to 
avoid the failure.
>
>>> This patch moves the framebuffer creation logic inside the
>>> modeset_with_fb() function. This ensures that for every modeset
>>> operation, a new framebuffer is created with dimensions that
>>> perfectly match the current output's mode, preventing potential
>>> modeset failures on systems with multiple displays of varying
>>> resolutions.
>> If we indeed see the issue and go ahead with this patch, fb cleanups
>> need to be handled for the additional fb's created. Currently we are
>> doing fb cleanup only once at the end of the test.
>>
>> Also we need to remove the existing fb creation as it becomes
>> redundant
>> with this change?
> Yes, I definitely agree with this.
> However, I was worried that it would break other tests in
> kms_content_protection, so I didn't remove the existing fb creation.
>
> Since data.red and data.green are static global variables, Isimply
> called igt_create_color_fb() again to overwrite the original fb, but
> this could indeed cause the potential problem of gem_handle not being
> properly released.
Yea this is not recommended to overwrite the fb pointer as it will cause 
leaks.
>
> Do you know if I replaced the original create_fbs() to here, where
> should the original igt_remove_fb() be moved to?

I think test_fini() should be the right place to handle this.

Regards,
Karthik.B.S
>
> Regards,
> Jason-JH Lin
>
>> Regards,
>> Karthik.B.S
>>