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(); }".
Thanks,
Paulo
> Otherwise I really like this.
> -Daniel
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> Another alternative would have been to change HAS_PCH_IBX to also
>> check for HAS_PCH_SPLIT, but I'm not exactly in favor of adding more
>> conditionals...
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index b7a1eaa..b12e79a 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -333,6 +333,7 @@ enum no_fbc_reason {
>> };
>>
>> enum intel_pch {
>> + PCH_NONE = 0, /* No PCH present */
>> PCH_IBX, /* Ibexpeak PCH */
>> PCH_CPT, /* Cougarpoint PCH */
>> PCH_LPT, /* Lynxpoint PCH */
>> --
>> 1.7.10.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Mail: daniel at ffwll.ch
> Mobile: +41 (0)79 365 57 48
--
Paulo Zanoni
