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@xxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx>
>
> /* 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
