| Message ID | 20231008175615.413497-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/dpu: Use the encoder for default CRC source | expand |
On Sun, 8 Oct 2023 at 20:56, Rob Clark <robdclark@gmail.com> wrote: > > From: Rob Clark <robdclark@chromium.org> > > i-g-t expects the CRC to reflect any applied CTM. But the layer mixer > source is upstream of the DSPP, so it is before the CTM is applied. > > Switch the default source to 'encoder' instead so that the CRC is > captured downstream of the DSPP. > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/ci/xfails/msm-sc7180-fails.txt | 4 ---- > drivers/gpu/drm/ci/xfails/msm-sc7180-skips.txt | 5 ----- > drivers/gpu/drm/ci/xfails/msm-sdm845-fails.txt | 11 +---------- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 +++--- I'm not sure, if updating the CI skip list together with the functional changs is a good idea, my preference would be towards two separate patches. Nevertheless: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > 4 files changed, 4 insertions(+), 22 deletions(-)
On 08/10/2023 16:59, Dmitry Baryshkov wrote: > On Sun, 8 Oct 2023 at 20:56, Rob Clark <robdclark@gmail.com> wrote: >> >> From: Rob Clark <robdclark@chromium.org> >> >> i-g-t expects the CRC to reflect any applied CTM. But the layer mixer >> source is upstream of the DSPP, so it is before the CTM is applied. >> >> Switch the default source to 'encoder' instead so that the CRC is >> captured downstream of the DSPP. >> >> Signed-off-by: Rob Clark <robdclark@chromium.org> >> --- >> drivers/gpu/drm/ci/xfails/msm-sc7180-fails.txt | 4 ---- >> drivers/gpu/drm/ci/xfails/msm-sc7180-skips.txt | 5 ----- >> drivers/gpu/drm/ci/xfails/msm-sdm845-fails.txt | 11 +---------- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 +++--- > > I'm not sure, if updating the CI skip list together with the > functional changs is a good idea, my preference would be towards two > separate patches. On the other hand, having both together in the same patch documents which tests this PATCH is fixing. Regards, Helen > > Nevertheless: > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> 4 files changed, 4 insertions(+), 22 deletions(-) > >
On Sun, Oct 8, 2023 at 4:21 PM Helen Koike <helen.koike@collabora.com> wrote: > > > > On 08/10/2023 16:59, Dmitry Baryshkov wrote: > > On Sun, 8 Oct 2023 at 20:56, Rob Clark <robdclark@gmail.com> wrote: > >> > >> From: Rob Clark <robdclark@chromium.org> > >> > >> i-g-t expects the CRC to reflect any applied CTM. But the layer mixer > >> source is upstream of the DSPP, so it is before the CTM is applied. > >> > >> Switch the default source to 'encoder' instead so that the CRC is > >> captured downstream of the DSPP. > >> > >> Signed-off-by: Rob Clark <robdclark@chromium.org> > >> --- > >> drivers/gpu/drm/ci/xfails/msm-sc7180-fails.txt | 4 ---- > >> drivers/gpu/drm/ci/xfails/msm-sc7180-skips.txt | 5 ----- > >> drivers/gpu/drm/ci/xfails/msm-sdm845-fails.txt | 11 +---------- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 +++--- > > > > I'm not sure, if updating the CI skip list together with the > > functional changs is a good idea, my preference would be towards two > > separate patches. > > On the other hand, having both together in the same patch documents > which tests this PATCH is fixing. on the mesa side of things, where we have more experience with CI, I've found updating expectations in same patch as code change to be hugely useful. I *might* end up dropping the xfails changes when I apply this patch (because there are some dependencies on igt uprev and I'm not sure we have enough CI runs to be confident about flakes) but I definitely believe that we should be updating expectations along with code changes. BR, -R > Regards, > Helen > > > > > Nevertheless: > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > >> 4 files changed, 4 insertions(+), 22 deletions(-) > > > >
diff --git a/drivers/gpu/drm/ci/xfails/msm-sc7180-fails.txt b/drivers/gpu/drm/ci/xfails/msm-sc7180-fails.txt index faf2702c223f..a51950746443 100644 --- a/drivers/gpu/drm/ci/xfails/msm-sc7180-fails.txt +++ b/drivers/gpu/drm/ci/xfails/msm-sc7180-fails.txt @@ -1,10 +1,6 @@ kms_cursor_legacy@cursor-vs-flip-toggle,Fail kms_cursor_legacy@cursor-vs-flip-varying-size,Fail kms_cursor_legacy@cursorA-vs-flipA-atomic-transitions,Crash -kms_pipe_crc_basic@compare-crc-sanitycheck-nv12,Fail -kms_plane_alpha_blend@alpha-7efc,Fail -kms_plane_alpha_blend@coverage-7efc,Fail -kms_plane_alpha_blend@coverage-vs-premult-vs-constant,Fail kms_plane_alpha_blend@pipe-A-alpha-7efc,Fail kms_plane_alpha_blend@pipe-A-coverage-7efc,Fail kms_plane_alpha_blend@pipe-A-coverage-vs-premult-vs-constant,Fail diff --git a/drivers/gpu/drm/ci/xfails/msm-sc7180-skips.txt b/drivers/gpu/drm/ci/xfails/msm-sc7180-skips.txt index 612f7e822c80..327039f70252 100644 --- a/drivers/gpu/drm/ci/xfails/msm-sc7180-skips.txt +++ b/drivers/gpu/drm/ci/xfails/msm-sc7180-skips.txt @@ -1,7 +1,2 @@ # Suspend to RAM seems to be broken on this machine .*suspend.* - -# Test incorrectly assumes that CTM support implies gamma/degamma -# LUT support. None of the subtests handle the case of only having -# CTM support -kms_color.* diff --git a/drivers/gpu/drm/ci/xfails/msm-sdm845-fails.txt b/drivers/gpu/drm/ci/xfails/msm-sdm845-fails.txt index ba36b92e3325..3d53c53a0659 100644 --- a/drivers/gpu/drm/ci/xfails/msm-sdm845-fails.txt +++ b/drivers/gpu/drm/ci/xfails/msm-sdm845-fails.txt @@ -1,20 +1,11 @@ kms_color@ctm-0-25,Fail kms_color@ctm-0-50,Fail -kms_color@ctm-0-75,Fail kms_color@ctm-blue-to-red,Fail kms_color@ctm-green-to-red,Fail kms_color@ctm-negative,Fail kms_color@ctm-red-to-blue,Fail kms_color@ctm-signed,Fail -kms_color@pipe-A-ctm-0-25,Fail -kms_color@pipe-A-ctm-0-5,Fail -kms_color@pipe-A-ctm-0-75,Fail -kms_color@pipe-A-ctm-blue-to-red,Fail -kms_color@pipe-A-ctm-green-to-red,Fail -kms_color@pipe-A-ctm-max,Fail -kms_color@pipe-A-ctm-negative,Fail -kms_color@pipe-A-ctm-red-to-blue,Fail -kms_color@pipe-A-legacy-gamma,Fail +kms_color@ctm-max,Fail kms_cursor_legacy@basic-flip-after-cursor-atomic,Fail kms_cursor_legacy@basic-flip-after-cursor-legacy,Fail kms_cursor_legacy@basic-flip-after-cursor-varying-size,Fail diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 8ce7586e2ddf..5eacf19382b5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -79,10 +79,10 @@ static enum dpu_crtc_crc_source dpu_crtc_parse_crc_source(const char *src_name) if (!src_name || !strcmp(src_name, "none")) return DPU_CRTC_CRC_SOURCE_NONE; - if (!strcmp(src_name, "auto") || - !strcmp(src_name, "lm")) + if (!strcmp(src_name, "lm")) return DPU_CRTC_CRC_SOURCE_LAYER_MIXER; - if (!strcmp(src_name, "encoder")) + if (!strcmp(src_name, "encoder") || + !strcmp(src_name, "auto")) return DPU_CRTC_CRC_SOURCE_ENCODER; return DPU_CRTC_CRC_SOURCE_INVALID;