[Intel-gfx] [PATCH 0/8] Haswell eDP enablement patches 1-8 v2

[Daniel Vetter]

On Sat, Oct 20, 2012 at 12:00 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Fri, Oct 19, 2012 at 11:19 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
>>       - Daniel suggested to not convert some of the code that touches FDI and
>>         the PCH transcoder but I did not remove these chunks since we're
>>         currently running this code on Haswell even when not using VGA, so if we
>>         don't convert the code to cpu_transcoder we might start poking the wrong
>>         pipes and messing a lot of things. I really don't think we should delay
>>         Haswell enablement even more based on cleanups we did not even write
>>         yet.  I've been maintaining these Haswell patches for a long time, I'd
>>         love to get rid of them. When we do the VGA cleanups we will be able to
>>         fix what we need to fix.
>
> Hm, where are we calling into fdi/pch code if the connector is not
> VGA? I've thought we've taken care of all these things by now ...

Ok, on irc Paulo clarified that in ironlake_crtc_disable we still call
some of the fdi and (pch) transcoder functions. Now for most cases I'd
simply call this a bug - we need to teach the code some smarts to
check whether any of the encoders are on the pch and only disable
things if that's the case.

But even without that consideration I don't think it makes sense to to
the s/pipe/cpu_transcoder transformation on these functions:
- At most we should talk about a separate pch transcoder  (to make the
vga encoder work on cpu_pipe != PIPE_A). The current code works
somewhat since there's only one pch transcoder on LPT, and by fixing
VGA to pipe A we make sure we don't try to use a non-existing pch
transcoder.
- We don't have an eDP transcoder on any pch device. So mapping to the
transcoder enum makes even less sense.

My proposal:
- ditch the s/pipe/cpu_transcoder/ that touch the fdi/pch transcoder
stuff from this patch series.
- when fixing up hsw vga support, first rename the current transcoder
functions to pch_transcoder, to make clear what they're dealing with
- add a intel_crtc->pch_transcoder, which for all platforms but hsw
would be the same as intel_crtc->pipe, switch all pch code over to use
that
- add an encoder->is_pch_encoder field, and wrap up all the fdi/pch
stuff with checks for this to avoid calling code that we should call.
We probably should keep around all the asserts that check whether
things are properly disabled.
- fix the pch_transcoder field on hsw to PIPE_A, since that's what
we'll ever use.

Depending upon what the code looks like, we could also create haswell
specific crtc_enable/disable functions and maybe even move all the
fdi/pch_transcoder stuff into the a hsw specific crt encoder (since
it's the only odd thing out).

Comments?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch