On Tue, Jul 3, 2012 at 10:29 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
> 2012/7/3 Daniel Vetter <daniel at ffwll.ch>:
>> I think most of the HAS_PCH_xxx are implicitly guarded because we've split
>> up the pch modeset into it's own functions. I think there might only be a
>> few issues in the encoder functions maybe. Have your checked all the
>> HAS_PCH_IBX checks there? If you want, I can go through the code, too.
>>
>
> I did check. At the moment we have just a few HAS_PCH_IBX calls in our
> driver. The only possible issues may be inside intel_hdmi.c and
> intel_dp.c (and they need more investigation).
>
> My biggest worry here is being "future-proof": are we sure whenever
> someone suggests adding HAS_PCH_IBX we'll remember that machines
> without a PCH return true for HAS_PCH_IBX? This is highly
> counter-intuitive. I really think that in future hardware enablement
> code we'll replace a lot of the "if (HAS_PCH_SPLIT) { foo(); } else {
> bar(); }" code for "if (HAS_PCH_NEW) { baz(); } else if (HAS_PCH_OLD)
> { foo(); } else { bar(); }".
Ok, I've quickly checked them. The one in intel_dp.c isn't an issue,
because DP is a gen5+ feature. So the only thing accidentally affected
is vlv, which isn't such a big deal ;-)
The only other check that isn't guarded with a HAS_PCH_SPLIT check is
in intel_hdmi.c for a ibx-only w/a. That one will also leak out into
gm45 platforms (which support hdmi, too).
Otherwise I haven't found anything. Can you please amend the commit
message detailing the effects on these two places? Just in case a
bisect hits this patch and someone is totally confused what's going on
here ...
-Daniel
--
Daniel Vetter
daniel.vetter at ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
