All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: relayout intel_panel.c to obey 80 char limit
@ 2013-09-16 21:48 Daniel Vetter
  2013-09-17  7:11 ` Jani Nikula
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2013-09-16 21:48 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

Especially intel_gmch_panel_fitting was shifting way too much over the
right edge and also was way too long. So extract two helpers, one for
gen4+ and one for gen2/3. Now the entire thing is again almost
readable ...

Spurred by checkpatch freaking out about a Ville's pipeconfig rework
in intel_panel.c

Otherwise just two lines that needed appropriate breaking.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_panel.c | 152 +++++++++++++++++++++----------------
 1 file changed, 88 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index c9dba46..e36149d 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -73,8 +73,10 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 	case DRM_MODE_SCALE_ASPECT:
 		/* Scale but preserve the aspect ratio */
 		{
-			u32 scaled_width = adjusted_mode->hdisplay * pipe_config->pipe_src_h;
-			u32 scaled_height = pipe_config->pipe_src_w * adjusted_mode->vdisplay;
+			u32 scaled_width = adjusted_mode->hdisplay
+				* pipe_config->pipe_src_h;
+			u32 scaled_height = pipe_config->pipe_src_w
+				* adjusted_mode->vdisplay;
 			if (scaled_width > scaled_height) { /* pillar */
 				width = scaled_height / pipe_config->pipe_src_h;
 				if (width & 1)
@@ -169,6 +171,83 @@ static inline u32 panel_fitter_scaling(u32 source, u32 target)
 	return (FACTOR * ratio + FACTOR/2) / FACTOR;
 }
 
+static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
+			      u32 *pfit_control)
+{
+	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
+	u32 scaled_width = adjusted_mode->hdisplay *
+		pipe_config->pipe_src_h;
+	u32 scaled_height = pipe_config->pipe_src_w *
+		adjusted_mode->vdisplay;
+
+	/* 965+ is easy, it does everything in hw */
+	if (scaled_width > scaled_height)
+		*pfit_control |= PFIT_ENABLE |
+			PFIT_SCALING_PILLAR;
+	else if (scaled_width < scaled_height)
+		*pfit_control |= PFIT_ENABLE |
+			PFIT_SCALING_LETTER;
+	else if (adjusted_mode->hdisplay != pipe_config->pipe_src_w)
+		*pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+}
+
+static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
+			      u32 *pfit_control, u32 *pfit_pgm_ratios,
+			      u32 *border)
+{
+	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
+	u32 scaled_width = adjusted_mode->hdisplay *
+		pipe_config->pipe_src_h;
+	u32 scaled_height = pipe_config->pipe_src_w *
+		adjusted_mode->vdisplay;
+	u32 bits;
+
+	/*
+	 * For earlier chips we have to calculate the scaling
+	 * ratio by hand and program it into the
+	 * PFIT_PGM_RATIO register
+	 */
+	if (scaled_width > scaled_height) { /* pillar */
+		centre_horizontally(adjusted_mode,
+				    scaled_height /
+				    pipe_config->pipe_src_h);
+
+		*border = LVDS_BORDER_ENABLE;
+		if (pipe_config->pipe_src_h != adjusted_mode->vdisplay) {
+			bits = panel_fitter_scaling(pipe_config->pipe_src_h,
+						    adjusted_mode->vdisplay);
+
+			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
+					     bits << PFIT_VERT_SCALE_SHIFT);
+			*pfit_control |= (PFIT_ENABLE |
+					  VERT_INTERP_BILINEAR |
+					  HORIZ_INTERP_BILINEAR);
+		}
+	} else if (scaled_width < scaled_height) { /* letter */
+		centre_vertically(adjusted_mode,
+				  scaled_width /
+				  pipe_config->pipe_src_w);
+
+		*border = LVDS_BORDER_ENABLE;
+		if (pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
+			bits = panel_fitter_scaling(pipe_config->pipe_src_w,
+						    adjusted_mode->hdisplay);
+
+			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
+					     bits << PFIT_VERT_SCALE_SHIFT);
+			*pfit_control |= (PFIT_ENABLE |
+					  VERT_INTERP_BILINEAR |
+					  HORIZ_INTERP_BILINEAR);
+		}
+	} else {
+		/* Aspects match, Let hw scale both directions */
+		*pfit_control |= (PFIT_ENABLE |
+				  VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
+				  VERT_INTERP_BILINEAR |
+				  HORIZ_INTERP_BILINEAR);
+	}
+}
+
 void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
@@ -196,67 +275,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 		break;
 	case DRM_MODE_SCALE_ASPECT:
 		/* Scale but preserve the aspect ratio */
-		if (INTEL_INFO(dev)->gen >= 4) {
-			u32 scaled_width = adjusted_mode->hdisplay *
-				pipe_config->pipe_src_h;
-			u32 scaled_height = pipe_config->pipe_src_w *
-				adjusted_mode->vdisplay;
-
-			/* 965+ is easy, it does everything in hw */
-			if (scaled_width > scaled_height)
-				pfit_control |= PFIT_ENABLE |
-					PFIT_SCALING_PILLAR;
-			else if (scaled_width < scaled_height)
-				pfit_control |= PFIT_ENABLE |
-					PFIT_SCALING_LETTER;
-			else if (adjusted_mode->hdisplay != pipe_config->pipe_src_w)
-				pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
-		} else {
-			u32 scaled_width = adjusted_mode->hdisplay *
-				pipe_config->pipe_src_h;
-			u32 scaled_height = pipe_config->pipe_src_w *
-				adjusted_mode->vdisplay;
-			/*
-			 * For earlier chips we have to calculate the scaling
-			 * ratio by hand and program it into the
-			 * PFIT_PGM_RATIO register
-			 */
-			if (scaled_width > scaled_height) { /* pillar */
-				centre_horizontally(adjusted_mode,
-						    scaled_height /
-						    pipe_config->pipe_src_h);
-
-				border = LVDS_BORDER_ENABLE;
-				if (pipe_config->pipe_src_h != adjusted_mode->vdisplay) {
-					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_h, adjusted_mode->vdisplay);
-					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
-							    bits << PFIT_VERT_SCALE_SHIFT);
-					pfit_control |= (PFIT_ENABLE |
-							 VERT_INTERP_BILINEAR |
-							 HORIZ_INTERP_BILINEAR);
-				}
-			} else if (scaled_width < scaled_height) { /* letter */
-				centre_vertically(adjusted_mode,
-						  scaled_width /
-						  pipe_config->pipe_src_w);
-
-				border = LVDS_BORDER_ENABLE;
-				if (pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
-					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_w, adjusted_mode->hdisplay);
-					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
-							    bits << PFIT_VERT_SCALE_SHIFT);
-					pfit_control |= (PFIT_ENABLE |
-							 VERT_INTERP_BILINEAR |
-							 HORIZ_INTERP_BILINEAR);
-				}
-			} else {
-				/* Aspects match, Let hw scale both directions */
-				pfit_control |= (PFIT_ENABLE |
-						 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
-						 VERT_INTERP_BILINEAR |
-						 HORIZ_INTERP_BILINEAR);
-			}
-		}
+		if (INTEL_INFO(dev)->gen >= 4)
+			i965_scale_aspect(pipe_config, &pfit_control);
+		else
+			i9xx_scale_aspect(pipe_config, &pfit_control,
+					  &pfit_pgm_ratios, &border);
 		break;
 	case DRM_MODE_SCALE_FULLSCREEN:
 		/*
@@ -438,7 +461,8 @@ static void intel_pch_panel_set_backlight(struct drm_device *dev, u32 level)
 	I915_WRITE(BLC_PWM_CPU_CTL, val | level);
 }
 
-static void intel_panel_actually_set_backlight(struct drm_device *dev, u32 level)
+static void intel_panel_actually_set_backlight(struct drm_device *dev,
+					       u32 level)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 tmp;
-- 
1.8.4.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: relayout intel_panel.c to obey 80 char limit
  2013-09-16 21:48 [PATCH] drm/i915: relayout intel_panel.c to obey 80 char limit Daniel Vetter
@ 2013-09-17  7:11 ` Jani Nikula
  2013-09-17  7:26   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Jani Nikula @ 2013-09-17  7:11 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter


Heh, I was reading the subject as relay-out, not re-layout for a while
there. -ENOCOFFEE.

On Tue, 17 Sep 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Especially intel_gmch_panel_fitting was shifting way too much over the
> right edge and also was way too long. So extract two helpers, one for
> gen4+ and one for gen2/3. Now the entire thing is again almost
> readable ...
>
> Spurred by checkpatch freaking out about a Ville's pipeconfig rework
> in intel_panel.c
>
> Otherwise just two lines that needed appropriate breaking.

I suppose it should be kind of obvious, but I do like the explicit "no
functional changes" in the commit message when none are intended.

With the reduced indent, you could have re-flowed some statements to
fewer lines while at it... but meh. Good stuff.

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

> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 152 +++++++++++++++++++++----------------
>  1 file changed, 88 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index c9dba46..e36149d 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -73,8 +73,10 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
>  	case DRM_MODE_SCALE_ASPECT:
>  		/* Scale but preserve the aspect ratio */
>  		{
> -			u32 scaled_width = adjusted_mode->hdisplay * pipe_config->pipe_src_h;
> -			u32 scaled_height = pipe_config->pipe_src_w * adjusted_mode->vdisplay;
> +			u32 scaled_width = adjusted_mode->hdisplay
> +				* pipe_config->pipe_src_h;
> +			u32 scaled_height = pipe_config->pipe_src_w
> +				* adjusted_mode->vdisplay;
>  			if (scaled_width > scaled_height) { /* pillar */
>  				width = scaled_height / pipe_config->pipe_src_h;
>  				if (width & 1)
> @@ -169,6 +171,83 @@ static inline u32 panel_fitter_scaling(u32 source, u32 target)
>  	return (FACTOR * ratio + FACTOR/2) / FACTOR;
>  }
>  
> +static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
> +			      u32 *pfit_control)
> +{
> +	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> +	u32 scaled_width = adjusted_mode->hdisplay *
> +		pipe_config->pipe_src_h;
> +	u32 scaled_height = pipe_config->pipe_src_w *
> +		adjusted_mode->vdisplay;
> +
> +	/* 965+ is easy, it does everything in hw */
> +	if (scaled_width > scaled_height)
> +		*pfit_control |= PFIT_ENABLE |
> +			PFIT_SCALING_PILLAR;
> +	else if (scaled_width < scaled_height)
> +		*pfit_control |= PFIT_ENABLE |
> +			PFIT_SCALING_LETTER;
> +	else if (adjusted_mode->hdisplay != pipe_config->pipe_src_w)
> +		*pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> +}
> +
> +static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
> +			      u32 *pfit_control, u32 *pfit_pgm_ratios,
> +			      u32 *border)
> +{
> +	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> +	u32 scaled_width = adjusted_mode->hdisplay *
> +		pipe_config->pipe_src_h;
> +	u32 scaled_height = pipe_config->pipe_src_w *
> +		adjusted_mode->vdisplay;
> +	u32 bits;
> +
> +	/*
> +	 * For earlier chips we have to calculate the scaling
> +	 * ratio by hand and program it into the
> +	 * PFIT_PGM_RATIO register
> +	 */
> +	if (scaled_width > scaled_height) { /* pillar */
> +		centre_horizontally(adjusted_mode,
> +				    scaled_height /
> +				    pipe_config->pipe_src_h);
> +
> +		*border = LVDS_BORDER_ENABLE;
> +		if (pipe_config->pipe_src_h != adjusted_mode->vdisplay) {
> +			bits = panel_fitter_scaling(pipe_config->pipe_src_h,
> +						    adjusted_mode->vdisplay);
> +
> +			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
> +					     bits << PFIT_VERT_SCALE_SHIFT);
> +			*pfit_control |= (PFIT_ENABLE |
> +					  VERT_INTERP_BILINEAR |
> +					  HORIZ_INTERP_BILINEAR);
> +		}
> +	} else if (scaled_width < scaled_height) { /* letter */
> +		centre_vertically(adjusted_mode,
> +				  scaled_width /
> +				  pipe_config->pipe_src_w);
> +
> +		*border = LVDS_BORDER_ENABLE;
> +		if (pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
> +			bits = panel_fitter_scaling(pipe_config->pipe_src_w,
> +						    adjusted_mode->hdisplay);
> +
> +			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
> +					     bits << PFIT_VERT_SCALE_SHIFT);
> +			*pfit_control |= (PFIT_ENABLE |
> +					  VERT_INTERP_BILINEAR |
> +					  HORIZ_INTERP_BILINEAR);
> +		}
> +	} else {
> +		/* Aspects match, Let hw scale both directions */
> +		*pfit_control |= (PFIT_ENABLE |
> +				  VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> +				  VERT_INTERP_BILINEAR |
> +				  HORIZ_INTERP_BILINEAR);
> +	}
> +}
> +
>  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  			      struct intel_crtc_config *pipe_config,
>  			      int fitting_mode)
> @@ -196,67 +275,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  		break;
>  	case DRM_MODE_SCALE_ASPECT:
>  		/* Scale but preserve the aspect ratio */
> -		if (INTEL_INFO(dev)->gen >= 4) {
> -			u32 scaled_width = adjusted_mode->hdisplay *
> -				pipe_config->pipe_src_h;
> -			u32 scaled_height = pipe_config->pipe_src_w *
> -				adjusted_mode->vdisplay;
> -
> -			/* 965+ is easy, it does everything in hw */
> -			if (scaled_width > scaled_height)
> -				pfit_control |= PFIT_ENABLE |
> -					PFIT_SCALING_PILLAR;
> -			else if (scaled_width < scaled_height)
> -				pfit_control |= PFIT_ENABLE |
> -					PFIT_SCALING_LETTER;
> -			else if (adjusted_mode->hdisplay != pipe_config->pipe_src_w)
> -				pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> -		} else {
> -			u32 scaled_width = adjusted_mode->hdisplay *
> -				pipe_config->pipe_src_h;
> -			u32 scaled_height = pipe_config->pipe_src_w *
> -				adjusted_mode->vdisplay;
> -			/*
> -			 * For earlier chips we have to calculate the scaling
> -			 * ratio by hand and program it into the
> -			 * PFIT_PGM_RATIO register
> -			 */
> -			if (scaled_width > scaled_height) { /* pillar */
> -				centre_horizontally(adjusted_mode,
> -						    scaled_height /
> -						    pipe_config->pipe_src_h);
> -
> -				border = LVDS_BORDER_ENABLE;
> -				if (pipe_config->pipe_src_h != adjusted_mode->vdisplay) {
> -					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_h, adjusted_mode->vdisplay);
> -					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
> -							    bits << PFIT_VERT_SCALE_SHIFT);
> -					pfit_control |= (PFIT_ENABLE |
> -							 VERT_INTERP_BILINEAR |
> -							 HORIZ_INTERP_BILINEAR);
> -				}
> -			} else if (scaled_width < scaled_height) { /* letter */
> -				centre_vertically(adjusted_mode,
> -						  scaled_width /
> -						  pipe_config->pipe_src_w);
> -
> -				border = LVDS_BORDER_ENABLE;
> -				if (pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
> -					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_w, adjusted_mode->hdisplay);
> -					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
> -							    bits << PFIT_VERT_SCALE_SHIFT);
> -					pfit_control |= (PFIT_ENABLE |
> -							 VERT_INTERP_BILINEAR |
> -							 HORIZ_INTERP_BILINEAR);
> -				}
> -			} else {
> -				/* Aspects match, Let hw scale both directions */
> -				pfit_control |= (PFIT_ENABLE |
> -						 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> -						 VERT_INTERP_BILINEAR |
> -						 HORIZ_INTERP_BILINEAR);
> -			}
> -		}
> +		if (INTEL_INFO(dev)->gen >= 4)
> +			i965_scale_aspect(pipe_config, &pfit_control);
> +		else
> +			i9xx_scale_aspect(pipe_config, &pfit_control,
> +					  &pfit_pgm_ratios, &border);
>  		break;
>  	case DRM_MODE_SCALE_FULLSCREEN:
>  		/*
> @@ -438,7 +461,8 @@ static void intel_pch_panel_set_backlight(struct drm_device *dev, u32 level)
>  	I915_WRITE(BLC_PWM_CPU_CTL, val | level);
>  }
>  
> -static void intel_panel_actually_set_backlight(struct drm_device *dev, u32 level)
> +static void intel_panel_actually_set_backlight(struct drm_device *dev,
> +					       u32 level)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 tmp;
> -- 
> 1.8.4.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: relayout intel_panel.c to obey 80 char limit
  2013-09-17  7:11 ` Jani Nikula
@ 2013-09-17  7:26   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2013-09-17  7:26 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel Graphics Development

On Tue, Sep 17, 2013 at 9:11 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> Heh, I was reading the subject as relay-out, not re-layout for a while
> there. -ENOCOFFEE.

Added a dash to the commit message.

> On Tue, 17 Sep 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> Especially intel_gmch_panel_fitting was shifting way too much over the
>> right edge and also was way too long. So extract two helpers, one for
>> gen4+ and one for gen2/3. Now the entire thing is again almost
>> readable ...
>>
>> Spurred by checkpatch freaking out about a Ville's pipeconfig rework
>> in intel_panel.c
>>
>> Otherwise just two lines that needed appropriate breaking.
>
> I suppose it should be kind of obvious, but I do like the explicit "no
> functional changes" in the commit message when none are intended.

Me too, but somehow I wanted to add it but then forgotten. Fixed.

> With the reduced indent, you could have re-flowed some statements to
> fewer lines while at it... but meh. Good stuff.
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Thanks for the review, patch slurped in.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-09-17  7:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-16 21:48 [PATCH] drm/i915: relayout intel_panel.c to obey 80 char limit Daniel Vetter
2013-09-17  7:11 ` Jani Nikula
2013-09-17  7:26   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.