Re: [PATCH 10/21] drm/i915: introduce lpt_enable_pch and cpt_enable_pch

[Eugeni Dodonov <eugeni.dodonov@linux.intel.com>]

On 07/04/2012 03:21 PM, Paulo Zanoni wrote:
> It looks like the check above could go on a separate commit. The first
> commit just moves code around, the second one adds the check.

This patch attempts to reset cpt_pch_enable to what ironlake_pch_enable
was before we added LPT support into it; and make lpt_pch_enable account
for LPT checks and such.

But yes, I can split it into several commits.

> I may have misunderstood something about this code which is not
> familiar to me, but instead of limiting everything to pipe 0, can't we
> just iterate through all pipes, read DDI_FUNC_CTL(pipe) and see if
> there's any other enabled pipe on FDI mode?

Yes, the idea in those asserts and checks was that as only 1 pipe can
work in FDI mode, and as we only have 1 FDI RX, 1 PCH Transcoder and so
on, we simple forced pipe1 to work in this mode, as this way most of
older code which uses macros still works.

But there is certainly a room for improvements.

>> -static void ironlake_pch_enable(struct drm_crtc *crtc)
>> +static void cpt_pch_enable(struct drm_crtc *crtc)
> 
> Since this will run on both IBX and CPT, shouldn't we call it
> ibx_pch_enable? At least on intel_hdmi.c, the functions are named
> after the earliest generation that can run them.

Yes, I can rename it to ibx_pch_enable.

I'll do those changes and split this one into smaller commits and will
resend later.

Thanks!
Eugeni