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.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > 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 <chris@xxxxxxxxxxxxxxxxxx>
Thanks.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
