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

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

[-- Attachment #1: Type: text/plain, Size: 3081 bytes --]

Hi,

On Thu, 2026-01-08 at 11:13 +0530, Karthik B S wrote:
> 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.
> 

OK, thanks for your advice.
I'll verify it and resend the next version.

Regard,
Jason-JH Lin

> Regards,
> Karthik.B.S
> 


[-- Attachment #2: Type: text/html, Size: 6814 bytes --]