From: "Kahola, Mika" <mika.kahola@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 06/20] drm/i915/fbc: Reuse the same struct for the cache and params
Date: Wed, 1 Dec 2021 10:00:49 +0000 [thread overview]
Message-ID: <8230f84fcf1444b7808c8a6b26694bda@intel.com> (raw)
In-Reply-To: <20211124113652.22090-7-ville.syrjala@linux.intel.com>
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, November 24, 2021 1:37 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 06/20] drm/i915/fbc: Reuse the same struct for the
> cache and params
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The FBC state cache and params are now nearly identical. Just use the same
> structure for both.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_fbc.c | 62 +++++++++++-------------
> drivers/gpu/drm/i915/i915_drv.h | 36 +++++---------
> 2 files changed, 40 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 1e8b75cdfad8..8625825cbee8 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -163,7 +163,7 @@ static u16 intel_fbc_override_cfb_stride(const struct
> intel_plane_state *plane_s
>
> static u32 i8xx_fbc_ctl(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
> unsigned int cfb_stride;
> u32 fbc_ctl;
> @@ -191,11 +191,11 @@ static u32 i8xx_fbc_ctl(struct intel_fbc *fbc)
>
> static u32 i965_fbc_ctl2(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> u32 fbc_ctl2;
>
> fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM |
> - FBC_CTL_PLANE(params->crtc.i9xx_plane);
> + FBC_CTL_PLANE(params->i9xx_plane);
>
> if (params->fence_id >= 0)
> fbc_ctl2 |= FBC_CTL_CPU_FENCE_EN;
> @@ -226,7 +226,7 @@ static void i8xx_fbc_deactivate(struct intel_fbc *fbc)
>
> static void i8xx_fbc_activate(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
> int i;
>
> @@ -258,8 +258,8 @@ static bool i8xx_fbc_is_compressing(struct intel_fbc
> *fbc)
>
> static void i8xx_fbc_nuke(struct intel_fbc *fbc) {
> - struct intel_fbc_reg_params *params = &fbc->params;
> - enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> + struct intel_fbc_state *params = &fbc->params;
> + enum i9xx_plane_id i9xx_plane = params->i9xx_plane;
> struct drm_i915_private *dev_priv = fbc->i915;
>
> spin_lock_irq(&dev_priv->uncore.lock);
> @@ -294,8 +294,8 @@ static const struct intel_fbc_funcs i8xx_fbc_funcs = {
>
> static void i965_fbc_nuke(struct intel_fbc *fbc) {
> - struct intel_fbc_reg_params *params = &fbc->params;
> - enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane;
> + struct intel_fbc_state *params = &fbc->params;
> + enum i9xx_plane_id i9xx_plane = params->i9xx_plane;
> struct drm_i915_private *dev_priv = fbc->i915;
>
> spin_lock_irq(&dev_priv->uncore.lock);
> @@ -330,12 +330,12 @@ static u32 g4x_dpfc_ctl_limit(struct intel_fbc *fbc)
>
> static u32 g4x_dpfc_ctl(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
> u32 dpfc_ctl;
>
> dpfc_ctl = g4x_dpfc_ctl_limit(fbc) |
> - DPFC_CTL_PLANE_G4X(params->crtc.i9xx_plane);
> + DPFC_CTL_PLANE_G4X(params->i9xx_plane);
>
> if (IS_G4X(i915))
> dpfc_ctl |= DPFC_CTL_SR_EN;
> @@ -352,7 +352,7 @@ static u32 g4x_dpfc_ctl(struct intel_fbc *fbc)
>
> static void g4x_fbc_activate(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
>
> intel_de_write(i915, DPFC_FENCE_YOFF,
> @@ -403,7 +403,7 @@ static const struct intel_fbc_funcs g4x_fbc_funcs = {
>
> static void ilk_fbc_activate(struct intel_fbc *fbc) {
> - struct intel_fbc_reg_params *params = &fbc->params;
> + struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
>
> intel_de_write(i915, ILK_DPFC_FENCE_YOFF, @@ -454,7 +454,7 @@
> static const struct intel_fbc_funcs ilk_fbc_funcs = {
>
> static void snb_fbc_program_fence(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
> u32 ctl = 0;
>
> @@ -491,7 +491,7 @@ static const struct intel_fbc_funcs snb_fbc_funcs = {
>
> static void glk_fbc_program_cfb_stride(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
> u32 val = 0;
>
> @@ -504,7 +504,7 @@ static void glk_fbc_program_cfb_stride(struct intel_fbc
> *fbc)
>
> static void skl_fbc_program_cfb_stride(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
> u32 val = 0;
>
> @@ -520,14 +520,14 @@ static void skl_fbc_program_cfb_stride(struct
> intel_fbc *fbc)
>
> static u32 ivb_dpfc_ctl(struct intel_fbc *fbc) {
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *params = &fbc->params;
> struct drm_i915_private *i915 = fbc->i915;
> u32 dpfc_ctl;
>
> dpfc_ctl = g4x_dpfc_ctl_limit(fbc);
>
> if (IS_IVYBRIDGE(i915))
> - dpfc_ctl |= DPFC_CTL_PLANE_IVB(params->crtc.i9xx_plane);
> + dpfc_ctl |= DPFC_CTL_PLANE_IVB(params->i9xx_plane);
>
> if (params->fence_id >= 0)
> dpfc_ctl |= DPFC_CTL_FENCE_EN_IVB;
> @@ -933,12 +933,14 @@ static void intel_fbc_update_state_cache(struct
> intel_atomic_state *state,
> const struct intel_plane_state *plane_state =
> intel_atomic_get_new_plane_state(state, plane);
> struct intel_fbc *fbc = plane->fbc;
> - struct intel_fbc_state_cache *cache = &fbc->state_cache;
> + struct intel_fbc_state *cache = &fbc->state_cache;
>
> cache->no_fbc_reason = plane_state->no_fbc_reason;
> if (cache->no_fbc_reason)
> return;
>
> + cache->i9xx_plane = plane->i9xx_plane;
> +
> /* FBC1 compression interval: arbitrary choice of 1 second */
> cache->interval = drm_mode_vrefresh(&crtc_state-
> >hw.adjusted_mode);
>
> @@ -1093,7 +1095,7 @@ static bool intel_fbc_can_activate(struct intel_crtc
> *crtc) {
> struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> struct intel_fbc *fbc = &i915->fbc;
> - struct intel_fbc_state_cache *cache = &fbc->state_cache;
> + struct intel_fbc_state *cache = &fbc->state_cache;
>
> if (!intel_fbc_can_enable(fbc))
> return false;
> @@ -1156,23 +1158,13 @@ static bool intel_fbc_can_activate(struct intel_crtc
> *crtc) static void intel_fbc_get_reg_params(struct intel_fbc *fbc,
> struct intel_crtc *crtc)
> {
> - const struct intel_fbc_state_cache *cache = &fbc->state_cache;
> - struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *cache = &fbc->state_cache;
> + struct intel_fbc_state *params = &fbc->params;
>
> /* Since all our fields are integer types, use memset here so the
> * comparison function can rely on memcmp because the padding will
> be
> * zero. */
> - memset(params, 0, sizeof(*params));
> -
> - params->fence_id = cache->fence_id;
> - params->fence_y_offset = cache->fence_y_offset;
> -
> - params->interval = cache->interval;
> - params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)-
> >i9xx_plane;
> -
> - params->cfb_stride = cache->cfb_stride;
> - params->cfb_size = cache->cfb_size;
> - params->override_cfb_stride = cache->override_cfb_stride;
> + *params = *cache;
> }
>
> static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state, @@ -
> 1188,8 +1180,8 @@ static bool intel_fbc_can_flip_nuke(struct
> intel_atomic_state *state,
> intel_atomic_get_new_plane_state(state, plane);
> const struct drm_framebuffer *old_fb = old_plane_state->hw.fb;
> const struct drm_framebuffer *new_fb = new_plane_state->hw.fb;
> - const struct intel_fbc_state_cache *cache = &fbc->state_cache;
> - const struct intel_fbc_reg_params *params = &fbc->params;
> + const struct intel_fbc_state *cache = &fbc->state_cache;
> + const struct intel_fbc_state *params = &fbc->params;
>
> if (drm_atomic_crtc_needs_modeset(&new_crtc_state->uapi))
> return false;
> @@ -1426,7 +1418,7 @@ static void intel_fbc_enable(struct intel_atomic_state
> *state,
> const struct intel_plane_state *plane_state =
> intel_atomic_get_new_plane_state(state, plane);
> struct intel_fbc *fbc = plane->fbc;
> - struct intel_fbc_state_cache *cache;
> + struct intel_fbc_state *cache;
> int min_limit;
>
> if (!fbc || !plane_state)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 66fa46d41fa5..a737fa483cf3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -403,6 +403,17 @@ struct intel_fbc_funcs;
>
> #define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address
> space */
>
> +struct intel_fbc_state {
> + const char *no_fbc_reason;
> + enum i9xx_plane_id i9xx_plane;
> + unsigned int cfb_stride;
> + unsigned int cfb_size;
> + unsigned int fence_y_offset;
> + u16 override_cfb_stride;
> + u16 interval;
> + s8 fence_id;
> +};
> +
> struct intel_fbc {
> struct drm_i915_private *i915;
> const struct intel_fbc_funcs *funcs;
> @@ -433,16 +444,7 @@ struct intel_fbc {
> * appropriate locking, so we cache information here in order to avoid
> * these problems.
> */
> - struct intel_fbc_state_cache {
> - const char *no_fbc_reason;
> -
> - unsigned int cfb_stride;
> - unsigned int cfb_size;
> - unsigned int fence_y_offset;
> - u16 override_cfb_stride;
> - u16 interval;
> - s8 fence_id;
> - } state_cache;
> + struct intel_fbc_state state_cache;
>
> /*
> * This structure contains everything that's relevant to program the @@
> -451,19 +453,7 @@ struct intel_fbc {
> * something different in the struct. The genx_fbc_activate functions
> * are supposed to read from it in order to program the registers.
> */
> - struct intel_fbc_reg_params {
> - struct {
> - enum i9xx_plane_id i9xx_plane;
> - } crtc;
> -
> - unsigned int cfb_stride;
> - unsigned int cfb_size;
> - unsigned int fence_y_offset;
> - u16 override_cfb_stride;
> - u16 interval;
> - s8 fence_id;
> - } params;
> -
> + struct intel_fbc_state params;
> const char *no_fbc_reason;
> };
>
> --
> 2.32.0
next prev parent reply other threads:[~2021-12-01 10:00 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-24 11:36 [Intel-gfx] [PATCH 00/20] drm/i915/fbc: More FBC refactoring Ville Syrjala
2021-11-24 11:36 ` [Intel-gfx] [PATCH 01/20] drm/i915/fbc: Eliminate racy intel_fbc_is_active() usage Ville Syrjala
2021-11-30 13:16 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 02/20] drm/i915/fbc: Pass whole plane state to intel_fbc_min_limit() Ville Syrjala
2021-11-30 13:17 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 03/20] drm/i915/fbc: Nuke lots of crap from intel_fbc_state_cache Ville Syrjala
2021-11-30 13:21 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 04/20] drm/i915/fbc: Relocate intel_fbc_override_cfb_stride() Ville Syrjala
2021-11-30 13:22 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 05/20] drm/i915/fbc: Nuke more FBC state Ville Syrjala
2021-12-01 9:44 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 06/20] drm/i915/fbc: Reuse the same struct for the cache and params Ville Syrjala
2021-12-01 10:00 ` Kahola, Mika [this message]
2021-11-24 11:36 ` [Intel-gfx] [PATCH 07/20] drm/i915/fbc: Pass around FBC instance instead of crtc Ville Syrjala
2021-12-01 10:03 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 08/20] drm/i915/fbc: Track FBC usage per-plane Ville Syrjala
2021-12-01 10:04 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 09/20] drm/i915/fbc: Flatten __intel_fbc_pre_update() Ville Syrjala
2021-12-01 10:04 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 10/20] drm/i915/fbc: Pass i915 instead of FBC instance to FBC underrun stuff Ville Syrjala
2021-12-01 10:08 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 11/20] drm/i915/fbc: Move FBC debugfs stuff into intel_fbc.c Ville Syrjala
2021-11-24 15:43 ` Jani Nikula
2021-11-25 9:43 ` Ville Syrjälä
2021-11-25 10:57 ` Jani Nikula
2021-11-25 12:13 ` Ville Syrjälä
2021-11-25 14:06 ` Tvrtko Ursulin
2021-11-25 14:27 ` Jani Nikula
2021-12-03 9:13 ` Ville Syrjälä
2021-12-03 9:55 ` Jani Nikula
2021-12-03 10:06 ` Ville Syrjälä
2021-12-03 10:47 ` Jani Nikula
2021-11-24 11:36 ` [Intel-gfx] [PATCH 12/20] drm/i915/fbc: Introduce intel_fbc_add_plane() Ville Syrjala
2021-12-01 10:40 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 13/20] drm/i915/fbc: Allocate intel_fbc dynamically Ville Syrjala
2021-12-01 11:02 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 14/20] drm/i915/fbc: Move stuff from intel_fbc_can_enable() into intel_fbc_check_plane() Ville Syrjala
2021-12-01 11:03 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 15/20] drm/i915/fbc: Disable FBC fully on FIFO underrun Ville Syrjala
2021-12-01 11:04 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 16/20] drm/i915/fbc: Nuke state_cache Ville Syrjala
2021-12-01 11:06 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 17/20] drm/i915/fbc: Move plane pointer into intel_fbc_state Ville Syrjala
2021-12-01 11:30 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 18/20] drm/i915/fbc: s/parms/fbc_state/ Ville Syrjala
2021-12-01 11:31 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 19/20] drm/i915/fbc: No FBC+double wide pipe Ville Syrjala
2021-12-01 11:32 ` Kahola, Mika
2021-11-24 11:36 ` [Intel-gfx] [PATCH 20/20] drm/i915/fbc: Pimp the FBC debugfs output Ville Syrjala
2021-12-03 11:48 ` Ville Syrjälä
2021-12-03 16:11 ` Jani Nikula
2021-11-24 13:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/fbc: More FBC refactoring Patchwork
2021-11-24 13:32 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-11-24 14:02 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-11-24 15:48 ` [Intel-gfx] [PATCH 00/20] " Jani Nikula
2021-11-26 6:48 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/fbc: More FBC refactoring (rev2) Patchwork
2021-11-26 6:49 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-11-26 7:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-11-26 9:01 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-11-28 6:08 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/fbc: More FBC refactoring (rev3) Patchwork
2021-11-28 6:09 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-11-28 6:42 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-11-28 8:22 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
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=8230f84fcf1444b7808c8a6b26694bda@intel.com \
--to=mika.kahola@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox