Re: [PATCH 1/4] drm/i915: remove palette_offsets from device info in favor of _PICK()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux