From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/4] drm/i915: remove palette_offsets from device info in favor of _PICK()
Date: Wed, 31 Oct 2018 14:59:44 +0200 [thread overview]
Message-ID: <20181031125944.GJ9144@intel.com> (raw)
In-Reply-To: <20181031110453.12722-1-jani.nikula@intel.com>
On Wed, Oct 31, 2018 at 01:04:50PM +0200, Jani Nikula wrote:
> The device info offset arrays for unevenly spaced register offsets is
> great for widely used registers. However, the palette registers are only
> used in one function, i9xx_load_luts_internal(), and only for GMCH
> platforms, wasting device info. Replace palette_offsets with _PICK() in
> palette register definition.
>
> While the use of _PICK() does not check for pipe C existence, neither
> does the current offset array usage, and leads to bogus address when
> pipe C is passed to PALETTE() on non-CHV. Using _PICK() at least leads
> to a sensible register offset, just non-existing on non-CHV. Either way,
> this shouldn't happen anyway.
>
> Remove unused old palette macros while at it.
>
> Bloat-o-meter results below for completeness.
>
> add/remove: 0/0 grow/shrink: 3/6 up/down: 94/-278 (-184)
> Function old new delta
> i9xx_load_luts_internal 394 483 +89
> i915_driver_load 5103 5107 +4
> g4x_pre_enable_dp 378 379 +1
> intel_engines_init_mmio 1117 1116 -1
> intel_engine_lookup_user 47 46 -1
> hdmi_port_clock_valid 310 309 -1
> gen11_irq_handler 707 706 -1
> intel_device_info_dump_runtime 329 311 -18
> intel_device_info_runtime_init 5166 4910 -256
> Total: Before=918650, After=918466, chg -0.02%
>
> add/remove: 0/0 grow/shrink: 0/48 up/down: 0/-576 (-576)
> Data old new delta
> intel_valleyview_info 200 188 -12
> intel_skylake_gt4_info 200 188 -12
> intel_skylake_gt3_info 200 188 -12
> intel_skylake_gt2_info 200 188 -12
> intel_skylake_gt1_info 200 188 -12
> intel_sandybridge_m_gt2_info 200 188 -12
> intel_sandybridge_m_gt1_info 200 188 -12
> intel_sandybridge_d_gt2_info 200 188 -12
> intel_sandybridge_d_gt1_info 200 188 -12
> intel_pineview_info 200 188 -12
> intel_kabylake_gt3_info 200 188 -12
> intel_kabylake_gt2_info 200 188 -12
> intel_kabylake_gt1_info 200 188 -12
> intel_ivybridge_q_info 200 188 -12
> intel_ivybridge_m_gt2_info 200 188 -12
> intel_ivybridge_m_gt1_info 200 188 -12
> intel_ivybridge_d_gt2_info 200 188 -12
> intel_ivybridge_d_gt1_info 200 188 -12
> intel_ironlake_m_info 200 188 -12
> intel_ironlake_d_info 200 188 -12
> intel_icelake_11_info 200 188 -12
> intel_i965gm_info 200 188 -12
> intel_i965g_info 200 188 -12
> intel_i945gm_info 200 188 -12
> intel_i945g_info 200 188 -12
> intel_i915gm_info 200 188 -12
> intel_i915g_info 200 188 -12
> intel_i865g_info 200 188 -12
> intel_i85x_info 200 188 -12
> intel_i845g_info 200 188 -12
> intel_i830_info 200 188 -12
> intel_haswell_gt3_info 200 188 -12
> intel_haswell_gt2_info 200 188 -12
> intel_haswell_gt1_info 200 188 -12
> intel_gm45_info 200 188 -12
> intel_geminilake_info 200 188 -12
> intel_g45_info 200 188 -12
> intel_g33_info 200 188 -12
> intel_coffeelake_gt3_info 200 188 -12
> intel_coffeelake_gt2_info 200 188 -12
> intel_coffeelake_gt1_info 200 188 -12
> intel_cherryview_info 200 188 -12
> intel_cannonlake_info 200 188 -12
> intel_broxton_info 200 188 -12
> intel_broadwell_rsvd_info 200 188 -12
> intel_broadwell_gt3_info 200 188 -12
> intel_broadwell_gt2_info 200 188 -12
> intel_broadwell_gt1_info 200 188 -12
> Total: Before=195529, After=194953, chg -0.29%
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_pci.c | 7 ++-----
> drivers/gpu/drm/i915/i915_reg.h | 16 +++++++---------
> drivers/gpu/drm/i915/intel_device_info.h | 1 -
> 3 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 44e745921ac1..4ccab8372dd4 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -36,16 +36,13 @@
> .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \
> .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> - TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET }, \
> - .palette_offsets = { PALETTE_A_OFFSET, PALETTE_B_OFFSET }
> + TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } \
>
> #define GEN_CHV_PIPEOFFSETS \
> .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> CHV_PIPE_C_OFFSET }, \
> .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> - CHV_TRANSCODER_C_OFFSET, }, \
> - .palette_offsets = { PALETTE_A_OFFSET, PALETTE_B_OFFSET, \
> - CHV_PALETTE_C_OFFSET }
> + CHV_TRANSCODER_C_OFFSET, } \
>
> #define CURSOR_OFFSETS \
> .cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_C_OFFSET }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bcee91bcfba6..d97cf98e3edf 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3439,11 +3439,13 @@ enum i915_power_well_id {
> /*
> * Palette regs
> */
> -#define PALETTE_A_OFFSET 0xa000
> -#define PALETTE_B_OFFSET 0xa800
> -#define CHV_PALETTE_C_OFFSET 0xc000
> -#define PALETTE(pipe, i) _MMIO(dev_priv->info.palette_offsets[pipe] + \
> - dev_priv->info.display_mmio_offset + (i) * 4)
> +#define _PALETTE_A 0xa000
> +#define _PALETTE_B 0xa800
> +#define _CHV_PALETTE_C 0xc000
> +#define PALETTE(pipe, i) _MMIO(dev_priv->info.display_mmio_offset + \
> + _PICK((pipe), _PALETTE_A, \
> + _PALETTE_B, _CHV_PALETTE_C) + \
> + (i) * 4)
Confused... Ah yes, we use a totally separate register define
for the legacy lut on non-gmch platforms. That explains why there's
nothing about those platforms in the patch.
Seems okay
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> /* MCH MMIO space */
>
> @@ -10645,10 +10647,6 @@ enum skl_power_gate {
> #define MIPI_READ_DATA_VALID(port) _MMIO_MIPI(port, _MIPIA_READ_DATA_VALID, _MIPIC_READ_DATA_VALID)
> #define READ_DATA_VALID(n) (1 << (n))
>
> -/* For UMS only (deprecated): */
> -#define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
> -#define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
> -
> /* MOCS (Memory Object Control State) registers */
> #define GEN9_LNCFCMOCS(i) _MMIO(0xb020 + (i) * 4) /* L3 Cache Control */
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index b4c2c4eae78b..86ce1db1b33a 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -177,7 +177,6 @@ struct intel_device_info {
> /* Register offsets for the various display pipes and transcoders */
> int pipe_offsets[I915_MAX_TRANSCODERS];
> int trans_offsets[I915_MAX_TRANSCODERS];
> - int palette_offsets[I915_MAX_PIPES];
> int cursor_offsets[I915_MAX_PIPES];
>
> /* Slice/subslice/EU info */
> --
> 2.11.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-10-31 12:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-31 11:04 [PATCH 1/4] drm/i915: remove palette_offsets from device info in favor of _PICK() Jani Nikula
2018-10-31 11:04 ` [PATCH 2/4] drm/i915: define _MMIO_PLANE() in terms of _PLANE() not _MMIO_PIPE() Jani Nikula
2018-10-31 13:00 ` Ville Syrjälä
2018-10-31 11:04 ` [PATCH 3/4] drm/i915: reorder and reindent the register choosing helper wrappers Jani Nikula
2018-10-31 13:02 ` Ville Syrjälä
2018-10-31 11:04 ` [PATCH 4/4] drm/i915: also group device info array helper macros with others Jani Nikula
2018-10-31 13:05 ` Ville Syrjälä
2018-10-31 12:57 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: remove palette_offsets from device info in favor of _PICK() Patchwork
2018-10-31 12:59 ` Ville Syrjälä [this message]
2018-11-01 17:09 ` Patchwork
2018-11-01 19:04 ` ✓ Fi.CI.IGT: " 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=20181031125944.GJ9144@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@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.