diff mbox

drm/i915: use the correct encoder type when comparing

Message ID 1344603910-4518-1-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Aug. 10, 2012, 1:05 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

  - intel_encoder->type is INTEL_OUTPUT_SOMETHING
  - drm_encoder->encoder_type is DRM_MODE_ENCODER_SOMETHING

Here we're using intel_encoder, so compare the oranges against
oranges. While at it, rename the variable to "intel_encoder" so we
keep our naming standards used everywhere.

Luckily this was not a bug because both DRM_MODE_ENCODER_DAC and
INTEL_OUTPUT_ANALOG are defined as 1. This is the only case where the
drm definition matches the intel definition.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

This was not tested, just compiled :)

Comments

Jani Nikula Aug. 10, 2012, 1:27 p.m. UTC | #1
On Fri, 10 Aug 2012, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
>   - intel_encoder->type is INTEL_OUTPUT_SOMETHING
>   - drm_encoder->encoder_type is DRM_MODE_ENCODER_SOMETHING
>
> Here we're using intel_encoder, so compare the oranges against
> oranges. While at it, rename the variable to "intel_encoder" so we
> keep our naming standards used everywhere.

intel_display.c still has plenty of 'struct intel_encoder *encoder'...

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Luckily this was not a bug because both DRM_MODE_ENCODER_DAC and
> INTEL_OUTPUT_ANALOG are defined as 1. This is the only case where the
> drm definition matches the intel definition.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> This was not tested, just compiled :)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2e1f28f..ea19a72 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2837,13 +2837,13 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
>  static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct intel_encoder *encoder;
> +	struct intel_encoder *intel_encoder;
>  
>  	/*
>  	 * If there's a non-PCH eDP on this crtc, it must be DP_A, and that
>  	 * must be driven by its own crtc; no sharing is possible.
>  	 */
> -	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +	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 */
> @@ -2851,19 +2851,19 @@ static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
>  			/* It is still unclear how this will work on PPT, so throw up a warning */
>  			WARN_ON(!HAS_PCH_LPT(dev));
>  
> -			if (encoder->type == DRM_MODE_ENCODER_DAC) {
> +			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",
> -						encoder->type);
> +					      intel_encoder->type);
>  				return false;
>  			}
>  		}
>  
> -		switch (encoder->type) {
> +		switch (intel_encoder->type) {
>  		case INTEL_OUTPUT_EDP:
> -			if (!intel_encoder_is_pch_edp(&encoder->base))
> +			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
>  				return false;
>  			continue;
>  		}
> -- 
> 1.7.11.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Aug. 10, 2012, 4:34 p.m. UTC | #2
On Fri, Aug 10, 2012 at 04:27:23PM +0300, Jani Nikula wrote:
> On Fri, 10 Aug 2012, Paulo Zanoni <przanoni@gmail.com> wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> >   - intel_encoder->type is INTEL_OUTPUT_SOMETHING
> >   - drm_encoder->encoder_type is DRM_MODE_ENCODER_SOMETHING
> >
> > Here we're using intel_encoder, so compare the oranges against
> > oranges. While at it, rename the variable to "intel_encoder" so we
> > keep our naming standards used everywhere.
> 
> intel_display.c still has plenty of 'struct intel_encoder *encoder'...
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Queued for -next, thanks for the patch.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2e1f28f..ea19a72 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2837,13 +2837,13 @@  static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
-	struct intel_encoder *encoder;
+	struct intel_encoder *intel_encoder;
 
 	/*
 	 * If there's a non-PCH eDP on this crtc, it must be DP_A, and that
 	 * must be driven by its own crtc; no sharing is possible.
 	 */
-	for_each_encoder_on_crtc(dev, crtc, encoder) {
+	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 */
@@ -2851,19 +2851,19 @@  static bool intel_crtc_driving_pch(struct drm_crtc *crtc)
 			/* It is still unclear how this will work on PPT, so throw up a warning */
 			WARN_ON(!HAS_PCH_LPT(dev));
 
-			if (encoder->type == DRM_MODE_ENCODER_DAC) {
+			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",
-						encoder->type);
+					      intel_encoder->type);
 				return false;
 			}
 		}
 
-		switch (encoder->type) {
+		switch (intel_encoder->type) {
 		case INTEL_OUTPUT_EDP:
-			if (!intel_encoder_is_pch_edp(&encoder->base))
+			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
 				return false;
 			continue;
 		}