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
> 

************* MEDIATEK Confidentiality Notice
 ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe
 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!