Re: [PATCH] drm/i915: Clean up device info structure definitions

Jani Nikula
2015-12-02T12:30:55Z

On Tue, 01 Dec 2015, Wayne Boyer <wayne.boyer@intel.com> wrote:
> Beginning with gen7, newer devices repetitively redefine values
> for the device info structure members. This patch simplifies the
> structure definitions by grouping member value definitions into the
> existing GEN7_FEATURES #define and into the new GEN7_LP_FEATURES
> and HSW_FEATURES #defines.
>
> Specifically, GEN_DEFAULT_PIPEOFFSETS and IVB_CURSOR_OFFSETS are
> added to GEN7_FEATURES and subsequent IVB definitions are simplified.
>
> GEN7_LP_FEATURES is created to differentiate and simplify the
> gen7 low power (LP) devices.

Not convinced on this one, maybe just go for VLV_FEATURES.

Otherwise seems fine.

BR,
Jani.

>
> HSW_FEATURES is created and used to simplify all HSW+ devices
> except for LP.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 137 ++++++++++------------------------------
> 1 file changed, 35 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 90faa8e..5f8b0ff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -226,125 +226,87 @@ static const struct intel_device_info intel_sandybridge_m_info = {
> #define GEN7_FEATURES \
> .gen = 7, .num_pipes = 3, \
> .need_gfx_hws = 1, .has_hotplug = 1, \
> .has_fbc = 1, \
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> - .has_llc = 1
> + .has_llc = 1, \
> + GEN_DEFAULT_PIPEOFFSETS, \
> + IVB_CURSOR_OFFSETS
>
> static const struct intel_device_info intel_ivybridge_d_info = {
> GEN7_FEATURES,
> .is_ivybridge = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_ivybridge_m_info = {
> GEN7_FEATURES,
> .is_ivybridge = 1,
> .is_mobile = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_ivybridge_q_info = {
> GEN7_FEATURES,
> .is_ivybridge = 1,
> .num_pipes = 0, /* legal, last one wins */
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> +#define GEN7_LP_FEATURES \
> + .gen = 7, .num_pipes = 2, \
> + .need_gfx_hws = 1, .has_hotplug = 1, \
> + .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> + .display_mmio_offset = VLV_DISPLAY_BASE, \
> + GEN_DEFAULT_PIPEOFFSETS, \
> + CURSOR_OFFSETS
> +
> static const struct intel_device_info intel_valleyview_m_info = {
> - GEN7_FEATURES,
> - .is_mobile = 1,
> - .num_pipes = 2,
> + GEN7_LP_FEATURES,
> .is_valleyview = 1,
> - .display_mmio_offset = VLV_DISPLAY_BASE,
> - .has_fbc = 0, /* legal, last one wins */
> - .has_llc = 0, /* legal, last one wins */
> - GEN_DEFAULT_PIPEOFFSETS,
> - CURSOR_OFFSETS,
> + .is_mobile = 1,
> };
>
> static const struct intel_device_info intel_valleyview_d_info = {
> - GEN7_FEATURES,
> - .num_pipes = 2,
> + GEN7_LP_FEATURES,
> .is_valleyview = 1,
> - .display_mmio_offset = VLV_DISPLAY_BASE,
> - .has_fbc = 0, /* legal, last one wins */
> - .has_llc = 0, /* legal, last one wins */
> - GEN_DEFAULT_PIPEOFFSETS,
> - CURSOR_OFFSETS,
> };
>
> +#define HSW_FEATURES \
> + GEN7_FEATURES, \
> + .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
> + .has_ddi = 1, \
> + .has_fpga_dbg = 1
> +
> static const struct intel_device_info intel_haswell_d_info = {
> - GEN7_FEATURES,
> + HSW_FEATURES,
> .is_haswell = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_haswell_m_info = {
> - GEN7_FEATURES,
> + HSW_FEATURES,
> .is_haswell = 1,
> .is_mobile = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_broadwell_d_info = {
> - .gen = 8, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> + HSW_FEATURES,
> + .gen = 8,
> };
>
> static const struct intel_device_info intel_broadwell_m_info = {
> - .gen = 8, .is_mobile = 1, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> + HSW_FEATURES,
> + .gen = 8, .is_mobile = 1,
> };
>
> static const struct intel_device_info intel_broadwell_gt3d_info = {
> - .gen = 8, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> + HSW_FEATURES,
> + .gen = 8,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_broadwell_gt3m_info = {
> - .gen = 8, .is_mobile = 1, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> + HSW_FEATURES,
> + .gen = 8, .is_mobile = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_cherryview_info = {
> .gen = 8, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> @@ -354,33 +316,19 @@ static const struct intel_device_info intel_cherryview_info = {
> GEN_CHV_PIPEOFFSETS,
> CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_skylake_info = {
> + HSW_FEATURES,
> .is_skylake = 1,
> - .gen = 9, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> + .gen = 9,
> };
>
> static const struct intel_device_info intel_skylake_gt3_info = {
> .is_skylake = 1,
> - .gen = 9, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> + .gen = 9,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_broxton_info = {
> .is_preliminary = 1,
> .is_broxton = 1,
> @@ -394,37 +342,22 @@ static const struct intel_device_info intel_broxton_info = {
> GEN_DEFAULT_PIPEOFFSETS,
> IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_kabylake_info = {
> + HSW_FEATURES,
> .is_preliminary = 1,
> .is_kabylake = 1,
> .gen = 9,
> - .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> static const struct intel_device_info intel_kabylake_gt3_info = {
> + HSW_FEATURES,
> .is_preliminary = 1,
> .is_kabylake = 1,
> .gen = 9,
> - .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> - .has_llc = 1,
> - .has_ddi = 1,
> - .has_fpga_dbg = 1,
> - .has_fbc = 1,
> - GEN_DEFAULT_PIPEOFFSETS,
> - IVB_CURSOR_OFFSETS,
> };
>
> /*
> * Make sure any device matches here are from most specific to most
> * general. For example, since the Quanta match is based on the subsystem

--
Jani Nikula, Intel Open Source Technology Center