From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id E15A089F4A for ; Wed, 10 Mar 2021 14:55:09 +0000 (UTC) References: <20210219172738.263820-1-jcline@redhat.com> <20210305164253.11788-1-jcline@redhat.com> <20210305164253.11788-2-jcline@redhat.com> <6ab3fa6c35e24f8b8f8cbcdfd271dfb7@intel.com> From: "Sharma, Swati2" Message-ID: Date: Wed, 10 Mar 2021 20:25:02 +0530 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Subject: Re: [igt-dev] [PATCH i-g-t v2 1/2] kms_hdr: Skip HDR tests on pre-Kaby Lake Intel devices List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Petri Latvala , "Shankar, Uma" Cc: "igt-dev@lists.freedesktop.org" List-ID: On 10-Mar-21 6:23 PM, Petri Latvala wrote: > On Wed, Mar 10, 2021 at 02:48:32PM +0200, Shankar, Uma wrote: >> >> >>> -----Original Message----- >>> From: Latvala, Petri >>> Sent: Tuesday, March 9, 2021 7:32 PM >>> To: Jeremy Cline >>> Cc: Sharma, Swati2 ; Maarten Lankhorst >>> ; igt-dev@lists.freedesktop.org; Shankar, Uma >>> >>> Subject: Re: [igt-dev] [PATCH i-g-t v2 1/2] kms_hdr: Skip HDR tests on pre-Kaby Lake >>> Intel devices >>> >>> On Mon, Mar 08, 2021 at 10:06:59AM -0500, Jeremy Cline wrote: >>>> On Mon, Mar 08, 2021 at 02:53:39PM +0200, Petri Latvala wrote: >>>>> On Mon, Mar 08, 2021 at 06:11:09PM +0530, Sharma, Swati2 wrote: >>>>>> >>>>>> >>>>>> On 08-Mar-21 2:17 PM, Petri Latvala wrote: >>>>>>> On Fri, Mar 05, 2021 at 11:42:52AM -0500, Jeremy Cline wrote: >>>>>>>> According to the Intel documentation[0] I could find, HDR >>>>>>>> support is only in Kaby Lake+. Skip tests in kms_hdr if the >>>>>>>> hardware doesn't support HDR. >>>>>>>> >>>>>>>> [0] >>>>>>>> https://www.intel.com/content/dam/support/us/en/documents/grap >>>>>>>> hics/HDR_Intel_Graphics_TechWhitePaper.pdf >>>>>>>> >>>>>>>> Signed-off-by: Jeremy Cline >>>>>>> >>>>>>> >>>>>>> While that might be true, strictly speaking IGT tests are not >>>>>>> testing the HW capabilities but the kernel interfaces. The >>>>>>> difference is often only interesting for nitpicking. >>>>>>> >>>>>>> However, in this case a good argument can be made either way, >>>>>>> with what the correct behaviour with setting the "max bpc" >>>>>>> property when the HW doesn't support HDR _output_ should be. IGT >>>>>>> tests should be written the way one would expect "real" >>>>>>> userspace to behave; does the documented kernel interface >>>>>>> require userspace to detect the device id somehow? The connector >>>>>>> property is there so one would assume setting it should work and do >>> something. >>>>>>> >>>>>>> A good argument can also be made that even though we're testing >>>>>>> "just the interface", we (Intel) should have a separate test >>>>>>> that requires actual HW support... >>>>>>> >>>>>>> Swati, Maarten, thoughts on this? Are we even testing the right >>>>>>> things for i915 at all? Are we able to express the HW >>>>>>> requirement for HDR with something other than comparing devid? >>>>>>> Should we? (If we should not, please suggest a better way to get >>>>>>> around the issue being fixed >>>>>>> here) >>>>>> >>>>>> There are 2 types of tests which are being validated in kms_hdr >>>>>> (i) bpc switch >>>>>> (ii) hdr metadata >>>>>> And both these tests will skip on platforms which doesn't support >>>>>> respective connector properties (MAX_BPC, HDR_OUTPUT_METADATA >>>>>> resp). These tests are independent of platforms on which they are >>>>>> being tested. >>>>>> This can be validated from the link below: >>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/shards-all.html?testfilte >>>>>> r=kms_hdr where you can see platforms which doesn't support either >>>>>> max_bpc or hdr_metadata connector properties; tests are being >>>>>> skipped. >>>>>> >>>>>> And I don't think anything is being fixed here >>>>> >>>>> What I meant is the fix in patch 2, removing the removal of the >>>>> primary plane, which was done because of a HSW limitation. Patch 1 >>>>> (this thread) is then making sure HSW is unaffected by a spurious >>>>> failure. Sorry for not being clear when pulling more CCs. >>>>> >>>> >>>> There are definitely finer-grain ways to do this, but this "works" so >>>> I figured it'd be good to start here and have a discussion about it. >>>> >>>> One option would be to just wrap the plane-removal call in a device >>>> check. Another would be to try and find a plane size that meets >>>> whatever the scaling requirements are for hsw (assuming there's >>>> overlap between the conflicting requirements of hardware). >>>> >>>> I don't have a strong opinion about where the checks happen, it seems >>>> like a trade-off between in-test complexity and the breadth of the >>>> test matrix, and I can't say how useful it is to make sure the MAX_BPC >>>> interface works on a specific generation of hardware. I'm happy to do >>>> either of those options (or another option I've not considered), >>>> whatever you folks think is the best trade-off. >>> >>> >>> Checked how HSW actually behaves with this and >>> >>> https://intel-gfx-ci.01.org/tree/drm-tip/TrybotIGT_300/shard- >>> hsw4/igt@kms_hdr@bpc-switch.html >>> >>> Swati, what's your opinion? if (amdgpudevice) around the plane removal or what's >>> best here? >> >> Hi Petri, >> I feel limiting it to amdgpudevice seems a safer and easier route here without disturbing the legacy behavior. > > Thanks, let's go with that then. > > Jeremy, see above. Yes, agree with Uma. > > -- ~Swati Sharma _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev