Hi
2012/10/25 Jani Nikula <jani.nikula at linux.intel.com>:
> On Tue, 23 Oct 2012, Paulo Zanoni <przanoni at gmail.com> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> By forking Ironlake and Haswell functions. The only callers are
>> {ironlake,haswell}_crtc_enable anyway, and this way we won't need to
>> add other checks on the Haswell version for the next gens.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++--------------------
>> 1 file changed, 11 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index a90da35..0c4e9c5 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2849,7 +2849,7 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
>> mutex_unlock(&dev->struct_mutex);
>> }
>>
>> -static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
>> +static bool ironlake_crtc_driving_pch(struct drm_crtc *crtc)
>> {
>> struct drm_device *dev = crtc->dev;
>> struct intel_encoder *intel_encoder;
>> @@ -2859,23 +2859,6 @@ static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
>> * must be driven by its own crtc; no sharing is possible.
>> */
>> for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
>> -
>> - /* On Haswell, LPT PCH handles the VGA connection via FDI, and Haswell
>> - * CPU handles all others */
>> - if (IS_HASWELL(dev)) {
>> - /* It is still unclear how this will work on PPT, so throw up a warning */
>> - WARN_ON(!HAS_PCH_LPT(dev));
>> -
>> - if (intel_encoder->type == INTEL_OUTPUT_ANALOG) {
>> - DRM_DEBUG_KMS("Haswell detected DAC encoder, assuming is PCH\n");
>> - return true;
>> - } else {
>> - DRM_DEBUG_KMS("Haswell detected encoder %d, assuming is CPU\n",
>> - intel_encoder->type);
>> - return false;
>> - }
>> - }
>> -
>> switch (intel_encoder->type) {
>> case INTEL_OUTPUT_EDP:
>> if (!intel_encoder_is_pch_edp(&intel_encoder->base))
>> @@ -2887,6 +2870,14 @@ static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
>> return true;
>> }
>>
>> +static bool haswell_crtc_driving_pch(struct drm_crtc *crtc)
>> +{
>> + if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG))
>> + return true;
>> + else
>> + return false;
>> +}
>
> Nitpick, this could be written just:
>
> return intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG);
Now my code looks so stupid! =)
>
> A pedantic observation: the previous version in intel_crtc_driving_pch
> unconditionally stopped at the first encoder on the crtc on HSW, this
> one checks that *none* of the encoders has type INTEL_OUTPUT_ANALOG
> before it returns false. It doesn't matter either way, does it?
I think the main reason for this patch is that I don't like the fact
that the old code unconditionally returned on the first iteration, but
that's not real a problem, it's just my opinion about coding style. On
Haswell there's just 1 encoder for each crtc and we check for this
condition on haswell_crtc_mode_set. On previous gens we can have more
than 1 encoder, so we have to loop over all the encoders.
>
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
>
>
>> +
>> /* Program iCLKIP clock to the desired frequency */
>> static void lpt_program_iclkip(struct drm_crtc *crtc)
>> {
>> @@ -3215,7 +3206,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>> I915_WRITE(PCH_LVDS, temp | LVDS_PORT_EN);
>> }
>>
>> - is_pch_port = intel_crtc_driving_pch(crtc);
>> + is_pch_port = ironlake_crtc_driving_pch(crtc);
>>
>> if (is_pch_port) {
>> ironlake_fdi_pll_enable(intel_crtc);
>> @@ -3293,7 +3284,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>> intel_crtc->active = true;
>> intel_update_watermarks(dev);
>>
>> - is_pch_port = intel_crtc_driving_pch(crtc);
>> + is_pch_port = haswell_crtc_driving_pch(crtc);
>>
>> if (is_pch_port) {
>> ironlake_fdi_pll_enable(intel_crtc);
>> --
>> 1.7.11.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
