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

Lucas De Marchi lucas.demarchi at intel.com
Wed Mar 6 18:55:01 UTC 2019


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.

checking now that that probably broke chv though.

Lucas De Marchi

>So using that to determine if a transcoder is present is not going
>to work.
>
>-- 
>Ville Syrjälä
>Intel


More information about the Intel-gfx mailing list