[v3,0/2] Enable Darkscreen Feature

Message

Nemesa Garg Dec. 13, 2023, 9:06 a.m. UTC

The logic checks for any black screen at pipe level and
upon such detection prints error. Darkscreen compares the
pixels with the compare value(0x00 - black) for the detection
purpose. This feature can be enables/disabled through debugfs.

Nemesa Garg (2):
  drm/i915/display: Add support for darskscreen detection
  drm/i915/display: Add darkscreen debugfs entry under crtc

 drivers/gpu/drm/i915/Makefile                 |   1 +
 .../gpu/drm/i915/display/intel_darkscreen.c   | 131 ++++++++++++++++++
 .../gpu/drm/i915/display/intel_darkscreen.h   |  26 ++++
 .../drm/i915/display/intel_display_debugfs.c  |   2 +
 .../drm/i915/display/intel_display_types.h    |   2 +
 drivers/gpu/drm/i915/i915_reg.h               |   8 ++
 drivers/gpu/drm/xe/Makefile                   |   1 +
 7 files changed, 171 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/display/intel_darkscreen.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_darkscreen.h

Comments

Jani Nikula Dec. 13, 2023, 9:58 a.m. UTC | #1

On Wed, 13 Dec 2023, Nemesa Garg <[e-mail]> wrote:
> The logic checks for any black screen at pipe level and
> upon such detection prints error. Darkscreen compares the
> pixels with the compare value(0x00 - black) for the detection
> purpose. This feature can be enables/disabled through debugfs.

This does not address the feedback I've given previously. Alas, it
wasn't on intel-gfx, so I'm copy-pasting it here:

> IGT patches https://patchwork.freedesktop.org/series/125880/ .
> Kernel patches https://patchwork.freedesktop.org/series/125563/ .

The current IGT implementation proposal does this:

 + igt_set_dark_screen_detection(data.drm_fd, pipe, true);
 + test_read_crc(&data, pipe, output, tests[i].flags);
 + igt_set_dark_screen_detection(data.drm_fd, pipe, false);

It *looks* nice. But the dark screen detection is *not* reported during
or after test_read_crc(). With the current kernel implementation, only
the dark screen enable checks if there's a dark screen during enable,
and that's it. The above checks what the state was before.

The kernel and the IGT parts don't work together. You need to come up
with a plan how the hardware feature can be used to our benefit. This
falls short, even for the first phase.

The detection is sticky, so you could fathom enabling it, and checking
later if dark screen has happened during testing. But if you enable it
before you have something on screen, you'll surely flag a dark screen
detection before you've even started the test. Right?

You need to have an idea how it's going to be used in a test case where
everything is disabled in the beginning.


BR,
Jani.


>
> Nemesa Garg (2):
>   drm/i915/display: Add support for darskscreen detection
>   drm/i915/display: Add darkscreen debugfs entry under crtc
>
>  drivers/gpu/drm/i915/Makefile                 |   1 +
>  .../gpu/drm/i915/display/intel_darkscreen.c   | 131 ++++++++++++++++++
>  .../gpu/drm/i915/display/intel_darkscreen.h   |  26 ++++
>  .../drm/i915/display/intel_display_debugfs.c  |   2 +
>  .../drm/i915/display/intel_display_types.h    |   2 +
>  drivers/gpu/drm/i915/i915_reg.h               |   8 ++
>  drivers/gpu/drm/xe/Makefile                   |   1 +
>  7 files changed, 171 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_darkscreen.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_darkscreen.h