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

[Ville Syrjälä]

On Wed, Mar 06, 2019 at 10:55:01AM -0800, Lucas De Marchi wrote:
> On Wed, Mar 06, 2019 at 07:55:33PM +0200, Ville Syrjälä wrote:
> >On Wed, Mar 06, 2019 at 09:42:57AM -0800, Lucas De Marchi wrote:
> >> On Wed, Mar 06, 2019 at 05:13:39PM +0200, Ville Syrjälä wrote:
> >> >On Wed, Mar 06, 2019 at 02:55:58PM +0000, Chris Wilson wrote:
> >> >> Quoting Ville Syrjälä (2019-03-06 14:52:11)
> >> >> > 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 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?
> >> >> >
> >> >> > What kind of knock on effect we're thinking?
> >> >>
> >> >> Just wondering why people are eager to make the assumption that
> >> >> non-existent pipes are not set. I presume its to make code neater.
> >> >>
> >> >> i.e. why cater to their whims at all?
> >> >
> >> >Yeah, I guess this was done just to avoid having convoluted platform
> >> >checks all over. I've not checked the code to see if there are
> >> >more places where we could simplify the existing code by adopting
> >> >this approach.
> >> >
> >> >However now that you forced me to think a bit I realize that this
> >> >may break in the presence of fused off pipes. Not quite sure how
> >> >the registers for such fused off blocks would behave. If we aren't
> >> >allowed to touch those registers we'd need to move this stuff
> >> >into the runtime info. That feels a bit wasteful, so as an
> >> >alternative we could just add one or two bitmasks instead.
> >> >
> >> >Cc:ing Lucas who seems to the main offender here...
> >>
> >> humn.. is this about the EDP transcoder? Missing some context here.
> >> Did I miss any platform not setting trans_offsets for TRANSCODER_EDP,
> >> even if it has? glancing over the possible mistake.... chv? :-/
> >
> >Currently .trans_offsets[TRANSCODER_EDP] != 0 on _every_ platform.
> 
> the commit was made to _allow_ platforms not to have the edp transcoder
> so we don't need to keep adding platform checks when the needs arrise.

Most of the platforms don't have EDP transcoder.

> 
> checking now that that probably broke chv though.

No, chv is fine. All pre-hsw platforms are busted though.

-- 
Ville Syrjälä
Intel