From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org,
Daniel Vetter <daniel.vetter@intel.com>, Lyude <cpaul@redhat.com>
Subject: Re: [Intel-gfx] [PATCH v4 0/6] Finally fix watermarks
Date: Fri, 29 Jul 2016 12:39:05 +0300 [thread overview]
Message-ID: <20160729093905.GU4329@intel.com> (raw)
In-Reply-To: <20160729000352.GR32025@intel.com>
On Thu, Jul 28, 2016 at 05:03:52PM -0700, Matt Roper wrote:
> This is completely untested (and probably horribly broken/buggy), but
> here's a quick mockup of the general approach I was thinking for
> ensuring DDB & WM's can be updated together while ensuring the
> three-step pipe flushing process is honored:
>
> https://github.com/mattrope/kernel/commits/experimental/lyude_ddb
>
> Basically the idea is to take note of what's happening to the pipe's DDB
> allocation (shrinking, growing, unchanged, etc.) during the atomic check
> phase;
Didn't look too closely, but I think you can't actually do that unless
you lock all the crtcs whenever the number of active pipes is goind to
change. Meaning we'd essentially be back to the one-big-modeset-lock
apporach, which will cause missed flips and whanot on the other pipes.
The alternative I think would consist of:
- make sure level 0 watermark never exceeds total_ddb_size/max_pipes,
so that a modeset doesn't have to care about the wms for the other
pipes not fitting in
- level 1+ watermarks would be checked against total_ddb_size
- protect the plane/pipe commit with the wm mutex whenever the wms
need to be reprogrammed
- keep the flush_wm thing around for the case when ddb size does get
changed, protect it with the wm lock
- when programming wms, we will first filter out any level that
doesn't fit in with the current ddb size, and then program the
rest in
- potentially introduce per-pipe wm locks if the one big lock looks
like an issue, which it might if the flush_wm holds it all the way
through
> then during the commit phase, we loop over the CRTC's three times
> instead of just once, but only operate on a subset of the CRTC's in each
> loop. While operating on each CRTC, the plane, WM, and DDB all get
> programmed together and have a single flush for all three.
>
>
>
>
> Matt
>
> On Tue, Jul 26, 2016 at 01:34:36PM -0400, Lyude wrote:
> > Latest version of https://lkml.org/lkml/2016/7/26/290 . Resending the whole
> > thing to keep it in one place.
> >
> > Lyude (5):
> > drm/i915/skl: Add support for the SAGV, fix underrun hangs
> > drm/i915/skl: Only flush pipes when we change the ddb allocation
> > drm/i915/skl: Fix extra whitespace in skl_flush_wm_values()
> > drm/i915/skl: Update plane watermarks atomically during plane updates
> > drm/i915/skl: Always wait for pipes to update after a flush
> >
> > Matt Roper (1):
> > drm/i915/gen9: Only copy WM results for changed pipes to skl_hw
> >
> > drivers/gpu/drm/i915/i915_drv.h | 3 +
> > drivers/gpu/drm/i915/i915_reg.h | 5 +
> > drivers/gpu/drm/i915/intel_display.c | 24 ++++
> > drivers/gpu/drm/i915/intel_drv.h | 4 +
> > drivers/gpu/drm/i915/intel_pm.c | 240 +++++++++++++++++++++++++++++++----
> > drivers/gpu/drm/i915/intel_sprite.c | 2 +
> > 6 files changed, 255 insertions(+), 23 deletions(-)
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: Lyude <cpaul@redhat.com>,
intel-gfx@lists.freedesktop.org,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
David Airlie <airlied@linux.ie>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [Intel-gfx] [PATCH v4 0/6] Finally fix watermarks
Date: Fri, 29 Jul 2016 12:39:05 +0300 [thread overview]
Message-ID: <20160729093905.GU4329@intel.com> (raw)
In-Reply-To: <20160729000352.GR32025@intel.com>
On Thu, Jul 28, 2016 at 05:03:52PM -0700, Matt Roper wrote:
> This is completely untested (and probably horribly broken/buggy), but
> here's a quick mockup of the general approach I was thinking for
> ensuring DDB & WM's can be updated together while ensuring the
> three-step pipe flushing process is honored:
>
> https://github.com/mattrope/kernel/commits/experimental/lyude_ddb
>
> Basically the idea is to take note of what's happening to the pipe's DDB
> allocation (shrinking, growing, unchanged, etc.) during the atomic check
> phase;
Didn't look too closely, but I think you can't actually do that unless
you lock all the crtcs whenever the number of active pipes is goind to
change. Meaning we'd essentially be back to the one-big-modeset-lock
apporach, which will cause missed flips and whanot on the other pipes.
The alternative I think would consist of:
- make sure level 0 watermark never exceeds total_ddb_size/max_pipes,
so that a modeset doesn't have to care about the wms for the other
pipes not fitting in
- level 1+ watermarks would be checked against total_ddb_size
- protect the plane/pipe commit with the wm mutex whenever the wms
need to be reprogrammed
- keep the flush_wm thing around for the case when ddb size does get
changed, protect it with the wm lock
- when programming wms, we will first filter out any level that
doesn't fit in with the current ddb size, and then program the
rest in
- potentially introduce per-pipe wm locks if the one big lock looks
like an issue, which it might if the flush_wm holds it all the way
through
> then during the commit phase, we loop over the CRTC's three times
> instead of just once, but only operate on a subset of the CRTC's in each
> loop. While operating on each CRTC, the plane, WM, and DDB all get
> programmed together and have a single flush for all three.
>
>
>
>
> Matt
>
> On Tue, Jul 26, 2016 at 01:34:36PM -0400, Lyude wrote:
> > Latest version of https://lkml.org/lkml/2016/7/26/290 . Resending the whole
> > thing to keep it in one place.
> >
> > Lyude (5):
> > drm/i915/skl: Add support for the SAGV, fix underrun hangs
> > drm/i915/skl: Only flush pipes when we change the ddb allocation
> > drm/i915/skl: Fix extra whitespace in skl_flush_wm_values()
> > drm/i915/skl: Update plane watermarks atomically during plane updates
> > drm/i915/skl: Always wait for pipes to update after a flush
> >
> > Matt Roper (1):
> > drm/i915/gen9: Only copy WM results for changed pipes to skl_hw
> >
> > drivers/gpu/drm/i915/i915_drv.h | 3 +
> > drivers/gpu/drm/i915/i915_reg.h | 5 +
> > drivers/gpu/drm/i915/intel_display.c | 24 ++++
> > drivers/gpu/drm/i915/intel_drv.h | 4 +
> > drivers/gpu/drm/i915/intel_pm.c | 240 +++++++++++++++++++++++++++++++----
> > drivers/gpu/drm/i915/intel_sprite.c | 2 +
> > 6 files changed, 255 insertions(+), 23 deletions(-)
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2016-07-29 9:39 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-26 17:34 [PATCH v4 0/6] Finally fix watermarks Lyude
2016-07-26 17:34 ` [PATCH v4 1/6] drm/i915/skl: Add support for the SAGV, fix underrun hangs Lyude
2016-07-26 17:34 ` Lyude
2016-07-28 13:13 ` Matt Roper
2016-07-28 13:13 ` Matt Roper
2016-08-02 11:16 ` [v4,1/6] " Hans de Goede
2016-08-02 11:16 ` Hans de Goede
2016-07-26 17:34 ` [PATCH v4 2/6] drm/i915/gen9: Only copy WM results for changed pipes to skl_hw Lyude
2016-07-26 17:34 ` Lyude
2016-07-26 17:34 ` [PATCH v4 3/6] drm/i915/skl: Only flush pipes when we change the ddb allocation Lyude
2016-07-28 13:14 ` Matt Roper
2016-07-28 13:14 ` Matt Roper
2016-07-26 17:34 ` [PATCH v4 4/6] drm/i915/skl: Fix extra whitespace in skl_flush_wm_values() Lyude
2016-07-26 17:34 ` Lyude
2016-07-26 17:34 ` [PATCH v4 5/6] drm/i915/skl: Update plane watermarks atomically during plane updates Lyude
2016-07-28 13:15 ` Matt Roper
2016-07-28 13:15 ` Matt Roper
2016-07-26 17:34 ` [PATCH v4 6/6] drm/i915/skl: Always wait for pipes to update after a flush Lyude
2016-07-27 5:40 ` ✗ Ro.CI.BAT: failure for Finally fix watermarks (rev2) Patchwork
2016-07-29 0:03 ` [PATCH v4 0/6] Finally fix watermarks Matt Roper
2016-07-29 0:03 ` [Intel-gfx] " Matt Roper
2016-07-29 9:39 ` Ville Syrjälä [this message]
2016-07-29 9:39 ` Ville Syrjälä
2016-07-29 18:48 ` Lyude
2016-07-29 18:48 ` [Intel-gfx] " Lyude
2016-07-29 19:26 ` Ville Syrjälä
2016-07-29 19:26 ` [Intel-gfx] " Ville Syrjälä
2016-07-29 19:46 ` Lyude
2016-07-29 19:46 ` [Intel-gfx] " Lyude
2016-07-29 20:41 ` Matt Roper
2016-07-29 20:41 ` [Intel-gfx] " Matt Roper
2016-08-01 11:50 ` Ville Syrjälä
2016-08-01 11:50 ` [Intel-gfx] " Ville Syrjälä
2016-07-29 20:33 ` Matt Roper
2016-07-29 20:33 ` [Intel-gfx] " Matt Roper
2016-08-01 8:48 ` Maarten Lankhorst
2016-08-01 8:48 ` [Intel-gfx] " Maarten Lankhorst
2016-08-01 11:48 ` Ville Syrjälä
2016-08-01 11:48 ` [Intel-gfx] " Ville Syrjälä
2016-08-02 15:41 ` Maarten Lankhorst
2016-08-02 15:41 ` Maarten Lankhorst
2016-08-02 15:59 ` Ville Syrjälä
2016-08-02 15:59 ` Ville Syrjälä
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160729093905.GU4329@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=cpaul@redhat.com \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matthew.d.roper@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.