Re: [Intel-gfx] [RFC 2/3] drm/i915: create intel_update_pipe_size()
On Tue, Sep 09, 2014 at 02:43:14PM -0300, Gustavo Padovan wrote: 2014-09-09 Ville Syrjälä ville.syrj...@linux.intel.com: On Tue, Sep 09, 2014 at 11:43:20AM -0300, Gustavo Padovan wrote: From: Gustavo Padovan gustavo.pado...@collabora.co.uk Factor out a piece of code from intel_pipe_set_base() that updates the pipe size and adjust fitter. This will help refactor the update primary plane path. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 71 +--- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2ccf7c0..e7e7184 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2779,6 +2779,46 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) return pending; } +static void intel_update_pipe_size(struct drm_crtc *crtc) These days we usually prefer to pass intel_crtc instead of drm_crtc. You can still call it 'crtc' since that's shorter and because we don't need anything from drm_crtc in this function there won't be any confusion between the two. Actually we need the drm_crtc 3 times in this function, that is why I left it as an argument. We could just do the other way around and get it from intel_crtc-base. Yeah I prefer we use the intel_foo types internally without an intel_ prefix in the local variable name. Adding the foo-base. prefix isn't really much longer than just foo. Imo upcasting should only be done in interface hooks where we have a reason for the paramater to have the more generic type, everywhere else it just looks a bit brittle. And yes I know that we're not there at all. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 2/3] drm/i915: create intel_update_pipe_size()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Factor out a piece of code from intel_pipe_set_base() that updates
the pipe size and adjust fitter.
This will help refactor the update primary plane path.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 71 +---
1 file changed, 41 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2ccf7c0..e7e7184 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2779,6 +2779,46 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc
*crtc)
return pending;
}
+static void intel_update_pipe_size(struct drm_crtc *crtc)
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ const struct drm_display_mode *adjusted_mode;
+
+ if (!i915.fastboot)
+ return;
+
+ /*
+* Update pipe size and adjust fitter if needed: the reason for this is
+* that in compute_mode_changes we check the native mode (not the pfit
+* mode) to see if we can flip rather than do a full mode set. In the
+* fastboot case, we'll flip, but if we don't update the pipesrc and
+* pfit state, we'll end up with a big fb scanned out into the wrong
+* sized surface.
+*
+* To fix this properly, we need to hoist the checks up into
+* compute_mode_changes (or above), check the actual pfit state and
+* whether the platform allows pfit disable with pipe active, and only
+* then update the pipesrc and pfit state, even on the flip path.
+*/
+
+ adjusted_mode = intel_crtc-config.adjusted_mode;
+
+ I915_WRITE(PIPESRC(intel_crtc-pipe),
+ ((adjusted_mode-crtc_hdisplay - 1) 16) |
+ (adjusted_mode-crtc_vdisplay - 1));
+ if (!intel_crtc-config.pch_pfit.enabled
+ (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
+intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
+ I915_WRITE(PF_CTL(intel_crtc-pipe), 0);
+ I915_WRITE(PF_WIN_POS(intel_crtc-pipe), 0);
+ I915_WRITE(PF_WIN_SZ(intel_crtc-pipe), 0);
+ }
+ intel_crtc-config.pipe_src_w = adjusted_mode-crtc_hdisplay;
+ intel_crtc-config.pipe_src_h = adjusted_mode-crtc_vdisplay;
+}
+
static int
intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *fb)
@@ -2821,36 +2861,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return ret;
}
- /*
-* Update pipe size and adjust fitter if needed: the reason for this is
-* that in compute_mode_changes we check the native mode (not the pfit
-* mode) to see if we can flip rather than do a full mode set. In the
-* fastboot case, we'll flip, but if we don't update the pipesrc and
-* pfit state, we'll end up with a big fb scanned out into the wrong
-* sized surface.
-*
-* To fix this properly, we need to hoist the checks up into
-* compute_mode_changes (or above), check the actual pfit state and
-* whether the platform allows pfit disable with pipe active, and only
-* then update the pipesrc and pfit state, even on the flip path.
-*/
- if (i915.fastboot) {
- const struct drm_display_mode *adjusted_mode =
- intel_crtc-config.adjusted_mode;
-
- I915_WRITE(PIPESRC(intel_crtc-pipe),
- ((adjusted_mode-crtc_hdisplay - 1) 16) |
- (adjusted_mode-crtc_vdisplay - 1));
- if (!intel_crtc-config.pch_pfit.enabled
- (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
-intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
- I915_WRITE(PF_CTL(intel_crtc-pipe), 0);
- I915_WRITE(PF_WIN_POS(intel_crtc-pipe), 0);
- I915_WRITE(PF_WIN_SZ(intel_crtc-pipe), 0);
- }
- intel_crtc-config.pipe_src_w = adjusted_mode-crtc_hdisplay;
- intel_crtc-config.pipe_src_h = adjusted_mode-crtc_vdisplay;
- }
+ intel_update_pipe_size(crtc);
dev_priv-display.update_primary_plane(crtc, fb, x, y);
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/3] drm/i915: create intel_update_pipe_size()
On Tue, Sep 09, 2014 at 11:43:20AM -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Factor out a piece of code from intel_pipe_set_base() that updates
the pipe size and adjust fitter.
This will help refactor the update primary plane path.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/i915/intel_display.c | 71
+---
1 file changed, 41 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2ccf7c0..e7e7184 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2779,6 +2779,46 @@ static bool intel_crtc_has_pending_flip(struct
drm_crtc *crtc)
return pending;
}
+static void intel_update_pipe_size(struct drm_crtc *crtc)
These days we usually prefer to pass intel_crtc instead of drm_crtc. You
can still call it 'crtc' since that's shorter and because we don't need
anything from drm_crtc in this function there won't be any confusion
between the two.
Otherwise the patch looks good.
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ const struct drm_display_mode *adjusted_mode;
+
+ if (!i915.fastboot)
+ return;
+
+ /*
+ * Update pipe size and adjust fitter if needed: the reason for this is
+ * that in compute_mode_changes we check the native mode (not the pfit
+ * mode) to see if we can flip rather than do a full mode set. In the
+ * fastboot case, we'll flip, but if we don't update the pipesrc and
+ * pfit state, we'll end up with a big fb scanned out into the wrong
+ * sized surface.
+ *
+ * To fix this properly, we need to hoist the checks up into
+ * compute_mode_changes (or above), check the actual pfit state and
+ * whether the platform allows pfit disable with pipe active, and only
+ * then update the pipesrc and pfit state, even on the flip path.
+ */
+
+ adjusted_mode = intel_crtc-config.adjusted_mode;
+
+ I915_WRITE(PIPESRC(intel_crtc-pipe),
+((adjusted_mode-crtc_hdisplay - 1) 16) |
+(adjusted_mode-crtc_vdisplay - 1));
+ if (!intel_crtc-config.pch_pfit.enabled
+ (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
+ intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
+ I915_WRITE(PF_CTL(intel_crtc-pipe), 0);
+ I915_WRITE(PF_WIN_POS(intel_crtc-pipe), 0);
+ I915_WRITE(PF_WIN_SZ(intel_crtc-pipe), 0);
+ }
+ intel_crtc-config.pipe_src_w = adjusted_mode-crtc_hdisplay;
+ intel_crtc-config.pipe_src_h = adjusted_mode-crtc_vdisplay;
+}
+
static int
intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *fb)
@@ -2821,36 +2861,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int
y,
return ret;
}
- /*
- * Update pipe size and adjust fitter if needed: the reason for this is
- * that in compute_mode_changes we check the native mode (not the pfit
- * mode) to see if we can flip rather than do a full mode set. In the
- * fastboot case, we'll flip, but if we don't update the pipesrc and
- * pfit state, we'll end up with a big fb scanned out into the wrong
- * sized surface.
- *
- * To fix this properly, we need to hoist the checks up into
- * compute_mode_changes (or above), check the actual pfit state and
- * whether the platform allows pfit disable with pipe active, and only
- * then update the pipesrc and pfit state, even on the flip path.
- */
- if (i915.fastboot) {
- const struct drm_display_mode *adjusted_mode =
- intel_crtc-config.adjusted_mode;
-
- I915_WRITE(PIPESRC(intel_crtc-pipe),
-((adjusted_mode-crtc_hdisplay - 1) 16) |
-(adjusted_mode-crtc_vdisplay - 1));
- if (!intel_crtc-config.pch_pfit.enabled
- (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
- intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
- I915_WRITE(PF_CTL(intel_crtc-pipe), 0);
- I915_WRITE(PF_WIN_POS(intel_crtc-pipe), 0);
- I915_WRITE(PF_WIN_SZ(intel_crtc-pipe), 0);
- }
- intel_crtc-config.pipe_src_w = adjusted_mode-crtc_hdisplay;
- intel_crtc-config.pipe_src_h = adjusted_mode-crtc_vdisplay;
- }
+ intel_update_pipe_size(crtc);
dev_priv-display.update_primary_plane(crtc, fb, x, y);
--
1.9.3
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
Re: [Intel-gfx] [RFC 2/3] drm/i915: create intel_update_pipe_size()
2014-09-09 Ville Syrjälä ville.syrj...@linux.intel.com: On Tue, Sep 09, 2014 at 11:43:20AM -0300, Gustavo Padovan wrote: From: Gustavo Padovan gustavo.pado...@collabora.co.uk Factor out a piece of code from intel_pipe_set_base() that updates the pipe size and adjust fitter. This will help refactor the update primary plane path. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 71 +--- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2ccf7c0..e7e7184 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2779,6 +2779,46 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) return pending; } +static void intel_update_pipe_size(struct drm_crtc *crtc) These days we usually prefer to pass intel_crtc instead of drm_crtc. You can still call it 'crtc' since that's shorter and because we don't need anything from drm_crtc in this function there won't be any confusion between the two. Actually we need the drm_crtc 3 times in this function, that is why I left it as an argument. We could just do the other way around and get it from intel_crtc-base. Gustavo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
