Re: [PATCH] drm/i915: Populate pipe_offsets[] & co. accurately

From: Lucas De Marchi <lucas.demarchi@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Populate pipe_offsets[] & co. accurately
Date: Wed, 6 Mar 2019 10:58:53 -0800	[thread overview]
Message-ID: <20190306185853.fnrmf5v6d5hqjyb5@ldmartin-desk.amr.corp.intel.com> (raw)
In-Reply-To: <20190306180105.GL3888@intel.com>

On Wed, Mar 06, 2019 at 08:01:05PM +0200, Ville Syrjälä 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@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.
>> So using that to determine if a transcoder is present is not going
>> to work.
>
>So far the HAS_EDP_TRANSCODER() thing is not actually a problem as
>it's only called from the ddi code which guarantees that the transcoder
>is in fact present. But the error capture code is now very much wrong.

Commit 062de72bc0c73f4b9fa8532e5a98bf609ebd08da was done in preparation
for HAS_EDP_TRANSCODER to account for that.

Lucas De Marchi

>
>-- 
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx