On Wed, Mar 06, 2019 at 09:31:48AM +0000, Chris Wilson wrote: > Quoting Ville Syrjala (2019-03-05 19:29:05) > > From: Ville Syrjälä <ville.syrj...@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?
What kind of knock on effect we're thinking? > > > Signed-off-by: Ville Syrjälä <ville.syrj...@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) 845g doesn't have dvo? The docs don't seem to quite agree with that. I should probably hunt one down just to complete my gen2 collection :) > so did it have a > second pipe? Though memory says dual pipe was a luxury of mobile only. Yeah, single pipe only I'm afraid. > > > 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. I rather dislike these template macros for this exact reason. > > And the device tables look to be using the correct feature sets. > > Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk> Thanks. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx