mbox series

[0/6] Cursor Fault Fixes

Message ID 20240205064836.3645402-1-chaitanya.kumar.borah@intel.com (mailing list archive)
Headers show
Series Cursor Fault Fixes | expand

Message

Chaitanya Kumar Borah Feb. 5, 2024, 6:48 a.m. UTC
This series is based on top of [1] floated by Maarten.
To fix regressions seen in CI, following changes were made on top of the
original series.

1. add a check in the plane state destroy hook to not move forward if the vblank worker is scheduled.
2. add checks before accessing frame buffer object (we are not sure yet how much this helps but we have found that this operation causes dump stacks)
3. do not defer the intel atomic cleanup into a work queue, instead execute it at the end of atomic commit tail.

While this series is in noway a complete or proper solution it is meant to trigger a discussion to arrive at one.

[1] https://patchwork.freedesktop.org/series/126934/

v2: Add missing patch
v3: Remove misleading error log
    Change condition to access fb object

Chaitanya Kumar Borah (3):
  drm/i915: do not destroy plane state if cursor unpin worker is
    scheduled
  drm/i915: Add sanity check before accessing fb buffer object
  drm/i915: do not defer cleanup work

Maarten Lankhorst (2):
  drm: Add drm_vblank_work_flush_all().
  drm/i915: Use the same vblank worker for atomic unpin

Ville Syrjälä (1):
  drm/i915: Use vblank worker to unpin old legacy cursor fb safely

 drivers/gpu/drm/drm_vblank_work.c             | 22 ++++++++
 .../gpu/drm/i915/display/intel_atomic_plane.c | 53 ++++++++++++++++++-
 .../gpu/drm/i915/display/intel_atomic_plane.h |  4 ++
 drivers/gpu/drm/i915/display/intel_crtc.c     | 27 ++++++++++
 drivers/gpu/drm/i915/display/intel_cursor.c   | 26 ++++++++-
 drivers/gpu/drm/i915/display/intel_cursor.h   |  3 ++
 drivers/gpu/drm/i915/display/intel_display.c  | 11 ++--
 .../drm/i915/display/intel_display_types.h    |  3 ++
 include/drm/drm_vblank_work.h                 |  2 +
 9 files changed, 142 insertions(+), 9 deletions(-)

Comments

Jani Nikula Feb. 5, 2024, 1:22 p.m. UTC | #1
On Mon, 05 Feb 2024, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
> This series is based on top of [1] floated by Maarten.
> To fix regressions seen in CI, following changes were made on top of the
> original series.

You've sent at least three versions of this in the past 24 hours, with
no indication of the version. Please use the -vN parameter in git
format-patch or git send-email to indicate the series version number in
patch subjects, so it's easier for people to track.

Thanks,
Jani.



>
> 1. add a check in the plane state destroy hook to not move forward if the vblank worker is scheduled.
> 2. add checks before accessing frame buffer object (we are not sure yet how much this helps but we have found that this operation causes dump stacks)
> 3. do not defer the intel atomic cleanup into a work queue, instead execute it at the end of atomic commit tail.
>
> While this series is in noway a complete or proper solution it is meant to trigger a discussion to arrive at one.
>
> [1] https://patchwork.freedesktop.org/series/126934/
>
> v2: Add missing patch
> v3: Remove misleading error log
>     Change condition to access fb object
>
> Chaitanya Kumar Borah (3):
>   drm/i915: do not destroy plane state if cursor unpin worker is
>     scheduled
>   drm/i915: Add sanity check before accessing fb buffer object
>   drm/i915: do not defer cleanup work
>
> Maarten Lankhorst (2):
>   drm: Add drm_vblank_work_flush_all().
>   drm/i915: Use the same vblank worker for atomic unpin
>
> Ville Syrjälä (1):
>   drm/i915: Use vblank worker to unpin old legacy cursor fb safely
>
>  drivers/gpu/drm/drm_vblank_work.c             | 22 ++++++++
>  .../gpu/drm/i915/display/intel_atomic_plane.c | 53 ++++++++++++++++++-
>  .../gpu/drm/i915/display/intel_atomic_plane.h |  4 ++
>  drivers/gpu/drm/i915/display/intel_crtc.c     | 27 ++++++++++
>  drivers/gpu/drm/i915/display/intel_cursor.c   | 26 ++++++++-
>  drivers/gpu/drm/i915/display/intel_cursor.h   |  3 ++
>  drivers/gpu/drm/i915/display/intel_display.c  | 11 ++--
>  .../drm/i915/display/intel_display_types.h    |  3 ++
>  include/drm/drm_vblank_work.h                 |  2 +
>  9 files changed, 142 insertions(+), 9 deletions(-)
Chaitanya Kumar Borah Feb. 5, 2024, 5:14 p.m. UTC | #2
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Monday, February 5, 2024 6:52 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar@intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>; maarten.lankhorst@linux.intel.com;
> ville.syrjala@linux.intel.com
> Subject: Re: [PATCH 0/6] Cursor Fault Fixes
> 
> On Mon, 05 Feb 2024, Chaitanya Kumar Borah
> <chaitanya.kumar.borah@intel.com> wrote:
> > This series is based on top of [1] floated by Maarten.
> > To fix regressions seen in CI, following changes were made on top of
> > the original series.
> 
> You've sent at least three versions of this in the past 24 hours, with no
> indication of the version. Please use the -vN parameter in git format-patch or
> git send-email to indicate the series version number in patch subjects, so it's
> easier for people to track.

Noted. For now, the reviewers can refer to the version history in the body of the cover letter. I will take care to add it into the subject as and when there is a next version.

Thank you.

Regards

Chaitanya

> 
> Thanks,
> Jani.
> 
> 
> 
> >
> > 1. add a check in the plane state destroy hook to not move forward if the
> vblank worker is scheduled.
> > 2. add checks before accessing frame buffer object (we are not sure
> > yet how much this helps but we have found that this operation causes
> dump stacks) 3. do not defer the intel atomic cleanup into a work queue,
> instead execute it at the end of atomic commit tail.
> >
> > While this series is in noway a complete or proper solution it is meant to
> trigger a discussion to arrive at one.
> >
> > [1] https://patchwork.freedesktop.org/series/126934/
> >
> > v2: Add missing patch
> > v3: Remove misleading error log
> >     Change condition to access fb object
> >
> > Chaitanya Kumar Borah (3):
> >   drm/i915: do not destroy plane state if cursor unpin worker is
> >     scheduled
> >   drm/i915: Add sanity check before accessing fb buffer object
> >   drm/i915: do not defer cleanup work
> >
> > Maarten Lankhorst (2):
> >   drm: Add drm_vblank_work_flush_all().
> >   drm/i915: Use the same vblank worker for atomic unpin
> >
> > Ville Syrjälä (1):
> >   drm/i915: Use vblank worker to unpin old legacy cursor fb safely
> >
> >  drivers/gpu/drm/drm_vblank_work.c             | 22 ++++++++
> >  .../gpu/drm/i915/display/intel_atomic_plane.c | 53
> > ++++++++++++++++++-  .../gpu/drm/i915/display/intel_atomic_plane.h |  4
> ++
> >  drivers/gpu/drm/i915/display/intel_crtc.c     | 27 ++++++++++
> >  drivers/gpu/drm/i915/display/intel_cursor.c   | 26 ++++++++-
> >  drivers/gpu/drm/i915/display/intel_cursor.h   |  3 ++
> >  drivers/gpu/drm/i915/display/intel_display.c  | 11 ++--
> >  .../drm/i915/display/intel_display_types.h    |  3 ++
> >  include/drm/drm_vblank_work.h                 |  2 +
> >  9 files changed, 142 insertions(+), 9 deletions(-)
> 
> --
> Jani Nikula, Intel