Re: [PATCH 00/19] [RFC] introduce struct intel_pipe_config

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed, 13 Feb 2013 12:32:03 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Hi all
> 
> So this is my old attempt to push our modeset infrastructure a bit further,
> rebased on top of latest dinq. Originally I wanted to push this a bit further
> still before submitting for rfc, but with fastboot picking up I think it's
> better to throw it out there now.
> 
> Roughly this adds a new intel_pipe_config struct which will (eventually) contain
> all the configuration parameters of an output pipe. With this I aim to solve a
> bunch of issues with the current code:
> 
> - We have a lot of encoder/output specific logic in our crtc code. Having a
>   struct to conveniently store flags and other special cases allows us to move
>   that logic into encoder callbacks. A few examples are bpp selection/dithering,
>   limited range selection, pixel clock computations, ... To have a few examples,
>   this series converts all users of the mode->flag and the bpp selection.

Yeah, I like this better than the mode flag stuff.  The new pipe bpp
chooser is definitely simpler, though now some bits are sprinkled to
the outputs.  I guess that's a bit better.

> - Shared resources are currently not checked or only after we've started to
>   change the hw state already. Examples are shared plls, shared fdi b/c lanes,
>   but also memory bw (we don't bother about this yet) and similar stuff. If the
>   code is restructured to compute the pipe config first and only then start
>   touching the hw, we can fix this. Fixing this is a requirement to get a
>   possible "check mode" of atomic modesetting working correctly.
> 
> - Intermingling of configuration selection and hw setup leads to inflexible
>   code. Best example is probably the bpp selection - some of the constraints
>   like fdi link bw are computed way too late, after e.g. the DP output has
>   computed link clock values already. So we can't adjust it any more and are
>   left with the only option to fail the modeset.
> 
>   Originally I've wanted to implement better handling of fdi b/c 2 lane
>   configurations as a proof-of-concept of this, hence just rfc. I'll try to
>   amend this over the next few days.
> 
> - Lacking infrastructure for fastboot hw state readout. I'm firmly of the
>   opinion that if we want fastboot to work on all the insane configuraions out
>   there, we need really solid hw state readout support. Hence this series also
>   adds pipe_config readout support and starts with some very minimal support for
>   comparing different such states.

Well, I think we'll have to punt in quite a few configs due to hw
limitations about what can be changed without a full pipe shutdown, but
we can definitely do better than we do today, and may be able to take
some shortcuts (provided we get lots of testing on the given hw).

> Comments, flames and ideas for the patches themselves, but also for the above
> issues in general highly welcome.

Overall I like this approach, it should make fastboot state tracking a
lot easier.

We'll also want to track gamma enable; unfortunately that can only be
changed with the plane off (according to docs), but we should check for
it regardless.

-- 
Jesse Barnes, Intel Open Source Technology Center