Message ID | 20231220121110.1441160-1-vignesh.raman@collabora.com (mailing list archive) |
---|---|
Headers | show |
Series | drm/ci: Add support for GPU and display testing | expand |
Hi, On Wed, 20 Dec 2023 at 12:11, Vignesh Raman <vignesh.raman@collabora.com> wrote: > Some ARM SOCs have a separate display controller and GPU, each with > different drivers. For mediatek mt8173, the GPU driver is powervr, > and the display driver is mediatek. In the case of mediatek mt8183, > the GPU driver is panfrost, and the display driver is mediatek. > With rockchip rk3288/rk3399, the GPU driver is panfrost, while the > display driver is rockchip. For amlogic meson, the GPU driver is > panfrost, and the display driver is meson. > > IGT tests run various tests with different xfails and can test both > GPU devices and KMS/display devices. Currently, in drm-ci for MediaTek, > Rockchip, and Amlogic Meson platforms, only the GPU driver is tested. > This leads to incomplete coverage since the display is never tested on > these platforms. This commit series adds support in drm-ci to run tests > for both GPU and display drivers for MediaTek, Rockchip, and Amlogic > Meson platforms. Thanks a lot for these. The patches need to be squashed to be bisectable though. For example, patch #2 changes the MTK job names and adds more jobs, but the corresponding xfail updates only come in #7 and #8. This means we have a span of a few patches where we don't have useful test results. A better sequencing would be something like: 1. add ANX7625 config 2. refactor _existing_ MTK display jobs to use YAML includes, change the existing job name, and rename the existing xfail set, remove IGT_FORCE_DRIVER from the script since it's now set by the job 3. add MTK Panfrost+PVR GPU jobs with new xfails, add xfail entry to MAINTAINERS 4+5: same as 2+3 but for Amlogic 6+7: same as 2+3 but for Rockchip Then the separate rename/update xfail commits just disappear, as does the removal of IGT_FORCE_DRIVER, because it's just done incrementally as part of the commits which change the related functionality. It's extremely important that every change can work standalone, instead of introducing intermediate breakage which is only fixed in later commits in the series. Cheers, Daniel
Hi Daniel, On 09/01/24 19:08, Daniel Stone wrote: > Hi, > > On Wed, 20 Dec 2023 at 12:11, Vignesh Raman <vignesh.raman@collabora.com> wrote: >> Some ARM SOCs have a separate display controller and GPU, each with >> different drivers. For mediatek mt8173, the GPU driver is powervr, >> and the display driver is mediatek. In the case of mediatek mt8183, >> the GPU driver is panfrost, and the display driver is mediatek. >> With rockchip rk3288/rk3399, the GPU driver is panfrost, while the >> display driver is rockchip. For amlogic meson, the GPU driver is >> panfrost, and the display driver is meson. >> >> IGT tests run various tests with different xfails and can test both >> GPU devices and KMS/display devices. Currently, in drm-ci for MediaTek, >> Rockchip, and Amlogic Meson platforms, only the GPU driver is tested. >> This leads to incomplete coverage since the display is never tested on >> these platforms. This commit series adds support in drm-ci to run tests >> for both GPU and display drivers for MediaTek, Rockchip, and Amlogic >> Meson platforms. > > Thanks a lot for these. The patches need to be squashed to be > bisectable though. For example, patch #2 changes the MTK job names and > adds more jobs, but the corresponding xfail updates only come in #7 > and #8. This means we have a span of a few patches where we don't have > useful test results. > > A better sequencing would be something like: > 1. add ANX7625 config > 2. refactor _existing_ MTK display jobs to use YAML includes, change > the existing job name, and rename the existing xfail set, remove > IGT_FORCE_DRIVER from the script since it's now set by the job > 3. add MTK Panfrost+PVR GPU jobs with new xfails, add xfail entry to > MAINTAINERS > 4+5: same as 2+3 but for Amlogic > 6+7: same as 2+3 but for Rockchip > > Then the separate rename/update xfail commits just disappear, as does > the removal of IGT_FORCE_DRIVER, because it's just done incrementally > as part of the commits which change the related functionality. It's > extremely important that every change can work standalone, instead of > introducing intermediate breakage which is only fixed in later commits > in the series. Thank you for reviewing the patches. I agree, will follow this sequence and send a v2 version. Regards, Vignesh
Hi Vignesh, On Wed, 10 Jan 2024 at 10:47, Vignesh Raman <vignesh.raman@collabora.com> wrote: > On 09/01/24 19:08, Daniel Stone wrote: > > A better sequencing would be something like: > > 1. add ANX7625 config > > 2. refactor _existing_ MTK display jobs to use YAML includes, change > > the existing job name, and rename the existing xfail set, remove > > IGT_FORCE_DRIVER from the script since it's now set by the job > > 3. add MTK Panfrost+PVR GPU jobs with new xfails, add xfail entry to > > MAINTAINERS > > 4+5: same as 2+3 but for Amlogic > > 6+7: same as 2+3 but for Rockchip > > > > Then the separate rename/update xfail commits just disappear, as does > > the removal of IGT_FORCE_DRIVER, because it's just done incrementally > > as part of the commits which change the related functionality. It's > > extremely important that every change can work standalone, instead of > > introducing intermediate breakage which is only fixed in later commits > > in the series. > > Thank you for reviewing the patches. I agree, will follow this sequence > and send a v2 version. Alongside Rob's patch to add msm-specific tests to the runlist, we'd need to add the Panfrost-specific tests. Whilst we're here, we might as well add the vc4/v3d/etnaviv/lima tests so they can use it in future. Panfrost should also skip kms_.* tests - since it's not a KMS driver, it can't run the KMS tests, so there's no point in trying. Cheers, Daniel
Hi Daniel, On 11/01/24 23:41, Daniel Stone wrote: > Hi Vignesh, > > On Wed, 10 Jan 2024 at 10:47, Vignesh Raman <vignesh.raman@collabora.com> wrote: >> On 09/01/24 19:08, Daniel Stone wrote: >>> A better sequencing would be something like: >>> 1. add ANX7625 config >>> 2. refactor _existing_ MTK display jobs to use YAML includes, change >>> the existing job name, and rename the existing xfail set, remove >>> IGT_FORCE_DRIVER from the script since it's now set by the job >>> 3. add MTK Panfrost+PVR GPU jobs with new xfails, add xfail entry to >>> MAINTAINERS >>> 4+5: same as 2+3 but for Amlogic >>> 6+7: same as 2+3 but for Rockchip >>> >>> Then the separate rename/update xfail commits just disappear, as does >>> the removal of IGT_FORCE_DRIVER, because it's just done incrementally >>> as part of the commits which change the related functionality. It's >>> extremely important that every change can work standalone, instead of >>> introducing intermediate breakage which is only fixed in later commits >>> in the series. >> >> Thank you for reviewing the patches. I agree, will follow this sequence >> and send a v2 version. > > Alongside Rob's patch to add msm-specific tests to the runlist, we'd > need to add the Panfrost-specific tests. Whilst we're here, we might > as well add the vc4/v3d/etnaviv/lima tests so they can use it in > future. > > Panfrost should also skip kms_.* tests - since it's not a KMS driver, > it can't run the KMS tests, so there's no point in trying. I will add these tests and update skips file. Sorry missed this before sending v2. I'm rechecking the xfails for the v2 series and will send v3 with these changes. Thanks. Regards, Vignesh