[Intel-gfx] [PATCH] drm/i915: Populate pipe_offsets[] & co. accurately

[Chris Wilson]

Quoting Ville Syrjala (2019-03-05 19:29:05)
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> At some point people have started to assume that
> pipe_offsets[] & co. are only populated for pipes and whatnot
> that actually exist. That is in fact not currently true, but
> we can easily make it so.

Any benefits of knock on effect?

> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c | 146 +++++++++++++++++++++++---------
>  1 file changed, 104 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index c42c5ccf38fe..9e610e4bdd7d 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -35,7 +35,37 @@
>  #define PLATFORM(x) .platform = (x), .platform_mask = BIT(x)
>  #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
>  
> -#define GEN_DEFAULT_PIPEOFFSETS \
> +#define I845_PIPE_OFFSETS \
> +       .pipe_offsets = { \
> +               [TRANSCODER_A] = PIPE_A_OFFSET, \
> +       }, \
> +       .trans_offsets = { \
> +               [TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> +       }
> +
> +#define I9XX_PIPE_OFFSETS \
> +       .pipe_offsets = { \
> +               [TRANSCODER_A] = PIPE_A_OFFSET, \
> +               [TRANSCODER_B] = PIPE_B_OFFSET, \
> +       }, \
> +       .trans_offsets = { \
> +               [TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> +               [TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> +       }
> +
> +#define IVB_PIPE_OFFSETS \
> +       .pipe_offsets = { \
> +               [TRANSCODER_A] = PIPE_A_OFFSET, \
> +               [TRANSCODER_B] = PIPE_B_OFFSET, \
> +               [TRANSCODER_C] = PIPE_C_OFFSET, \
> +       }, \
> +       .trans_offsets = { \
> +               [TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> +               [TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> +               [TRANSCODER_C] = TRANSCODER_C_OFFSET, \
> +       }
> +
> +#define HSW_PIPE_OFFSETS \
>         .pipe_offsets = { \
>                 [TRANSCODER_A] = PIPE_A_OFFSET, \
>                 [TRANSCODER_B] = PIPE_B_OFFSET, \
> @@ -49,7 +79,7 @@
>                 [TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
>         }
>  
> -#define GEN_CHV_PIPEOFFSETS \
> +#define CHV_PIPE_OFFSETS \
>         .pipe_offsets = { \
>                 [TRANSCODER_A] = PIPE_A_OFFSET, \
>                 [TRANSCODER_B] = PIPE_B_OFFSET, \
> @@ -61,11 +91,30 @@
>                 [TRANSCODER_C] = CHV_TRANSCODER_C_OFFSET, \
>         }
>  
> -#define CURSOR_OFFSETS \
> -       .cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_C_OFFSET }
> +#define I845_CURSOR_OFFSETS \
> +       .cursor_offsets = { \
> +               [PIPE_A] = CURSOR_A_OFFSET, \
> +       }
> +
> +#define I9XX_CURSOR_OFFSETS \
> +       .cursor_offsets = { \
> +               [PIPE_A] = CURSOR_A_OFFSET, \
> +               [PIPE_B] = CURSOR_B_OFFSET, \
> +       }
> +
> +#define CHV_CURSOR_OFFSETS \
> +       .cursor_offsets = { \
> +               [PIPE_A] = CURSOR_A_OFFSET, \
> +               [PIPE_B] = CURSOR_B_OFFSET, \
> +               [PIPE_C] = CHV_CURSOR_C_OFFSET, \
> +       }
>  
>  #define IVB_CURSOR_OFFSETS \
> -       .cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET }
> +       .cursor_offsets = { \
> +               [PIPE_A] = CURSOR_A_OFFSET, \
> +               [PIPE_B] = IVB_CURSOR_B_OFFSET, \
> +               [PIPE_C] = IVB_CURSOR_C_OFFSET, \
> +       }

Ok, those tables all match up with my understanding and rummaging.

>  static const struct intel_device_info intel_i865g_info = {
> -       GEN2_FEATURES,
> +       I845_FEATURES,

Hmm, 865g had dvo in addition to vga (unlike 845g) so did it have a
second pipe? Though memory says dual pipe was a luxury of mobile only.

>         PLATFORM(INTEL_I865G),
>  };
>  #define G75_FEATURES  \
> @@ -404,6 +465,7 @@ static const struct intel_device_info intel_valleyview_info = {

Grr.

>         .display.has_psr = 1, \
>         .display.has_dp_mst = 1, \
>         .has_rc6p = 0 /* RC6p removed-by HSW */, \
> +       HSW_PIPE_OFFSETS, \

Was thinking vlv using HSW? but then realised that diff was misleading.

And the device tables look to be using the correct feature sets.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris