Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Mon, Apr 27, 2015 at 05:13:49AM +, Konduru, Chandra wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Thursday, April 23, 2015 1:20 PM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: Matt Roper matthew.d.ro...@intel.com Testcase: kms_plane_scaling I wanted to pull this in but then spotted an issue. Since this needs one more round can you perhaps use the older version as a baseline again (without the refactoring)? Since skl planes aren't converted to universal planes yet it might be better to wait with refactoring until that's done actually. Two more comments below. Hi Daniel, Sorry for delay due to osts/f2f travel. Per last discussion at osts, you were ok to merge the changes made to skl update_plane functions to reduce its size. To undo these changes I have to redo all testing and also have to redo changes to upcoming nv12 changes. skl planes are already universal planes, Matt can comment more on this. But irrespective of whether skl planes universal or not with below changes, function sizes are more manageable. Yeah I looked at splitting up the refactoring, but because of the divergent history of the primary and sprite planes (which converged now in the hw for skl) it was messier than your patches here. Agreed that this is the exception which confirms the rule ;-) Regarding the FIXME and lock issue, will send out updated patch shortly. Thanks, both applied to dinq. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Wed, Apr 29, 2015 at 03:12:58PM +0300, Jani Nikula wrote: On Mon, 27 Apr 2015, Chandra Konduru chandra.kond...@intel.com wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. v6: -Added fixme note when checking plane_state-src width in update_plane (Daniel) -Release lock when failing to colorkey request with active scaler (Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: matthew.d.ro...@intel.com Matt, I never saw an explicit r-b from you for this patch, and there were also plenty of recent changes. Does your r-b still hold? Sorry for the slow response; I've been away from email for a few days. Yeah, I think I gave my r-b on IRC a while back, but suggested that Sonika also do a review since the merge conflicts were with her code. It looks like she's given her r-b as well, so I think this is good to go. Reviewed-by: Matt Roper matthew.d.ro...@intel.com Thanks. Matt Chandra, an earlier version of the patch has Testcase: kms_plane_scaling but this one doesn't. Does that test now do what Daniel requested in [1]? BR, Jani. [1] http://mid.gmane.org/20150413181245.GC6092@phenom.ffwll.local Reviewed-by: sonika.jin...@intel.com (v5) --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 266 +- drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c | 10 ++ 4 files changed, 219 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..22799fb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,204 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, -struct drm_framebuffer *fb, -int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF(pipe, 0), 0); - POSTING_READ(PLANE_CTL(pipe, 0)); - return; - } - - plane_ctl = PLANE_CTL_ENABLE | - PLANE_CTL_PIPE_GAMMA_ENABLE | - PLANE_CTL_PIPE_CSC_ENABLE; - - switch (fb-pixel_format) { + u32 plane_ctl_format = 0; + switch (pixel_format) { case DRM_FORMAT_RGB565: - plane_ctl |= PLANE_CTL_FORMAT_RGB_565; - break; - case DRM_FORMAT_XRGB: - plane_ctl |=
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Mon, 27 Apr 2015, Chandra Konduru chandra.kond...@intel.com wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. v6: -Added fixme note when checking plane_state-src width in update_plane (Daniel) -Release lock when failing to colorkey request with active scaler (Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: matthew.d.ro...@intel.com Matt, I never saw an explicit r-b from you for this patch, and there were also plenty of recent changes. Does your r-b still hold? Chandra, an earlier version of the patch has Testcase: kms_plane_scaling but this one doesn't. Does that test now do what Daniel requested in [1]? BR, Jani. [1] http://mid.gmane.org/20150413181245.GC6092@phenom.ffwll.local Reviewed-by: sonika.jin...@intel.com (v5) --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 266 +- drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c | 10 ++ 4 files changed, 219 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..22799fb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,204 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF(pipe, 0), 0); - POSTING_READ(PLANE_CTL(pipe, 0)); - return; - } - - plane_ctl = PLANE_CTL_ENABLE | - PLANE_CTL_PIPE_GAMMA_ENABLE | - PLANE_CTL_PIPE_CSC_ENABLE; - - switch (fb-pixel_format) { + u32 plane_ctl_format = 0; + switch (pixel_format) { case DRM_FORMAT_RGB565: - plane_ctl |= PLANE_CTL_FORMAT_RGB_565; - break; - case DRM_FORMAT_XRGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - break; - case DRM_FORMAT_ARGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY; + plane_ctl_format = PLANE_CTL_FORMAT_RGB_565; break; case DRM_FORMAT_XBGR: - plane_ctl |= PLANE_CTL_ORDER_RGBX; - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; + plane_ctl_format =
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
-Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Wednesday, April 29, 2015 5:13 AM To: Konduru, Chandra; intel-gfx@lists.freedesktop.org; Roper, Matthew D Cc: Vetter, Daniel; Conselvan De Oliveira, Ander Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers On Mon, 27 Apr 2015, Chandra Konduru chandra.kond...@intel.com wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. v6: -Added fixme note when checking plane_state-src width in update_plane (Daniel) -Release lock when failing to colorkey request with active scaler (Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: matthew.d.ro...@intel.com Matt, I never saw an explicit r-b from you for this patch, and there were also plenty of recent changes. Does your r-b still hold? Chandra, an earlier version of the patch has Testcase: kms_plane_scaling but this one doesn't. Does that test now do what Daniel requested in [1]? Hi Jani, Yes, same kms_plane_scaling is applicable. In patch creation, missed to add back the line: Testcase: kms_plane_scaling Regarding review, Matt reviewed until v4. For v5: changes are related to rebasing for 90/270, and Matt's suggestion was to get it reviewed from 90/270 owner (Sonika). Got R-b for v5 changes from her. For v6- it is adding a comment and addressing lock issue; nothing new added. I have sent a patch to update kms_plane_scaling but need some minor updates to address review feedback. I think, that shouldn't block merge of remaining scaler patches. BR, Jani. [1] http://mid.gmane.org/20150413181245.GC6092@phenom.ffwll.local Reviewed-by: sonika.jin...@intel.com (v5) --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 266 + - drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c | 10 ++ 4 files changed, 219 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..22799fb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,204 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, -struct drm_framebuffer *fb, -int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF
[Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. v6: -Added fixme note when checking plane_state-src width in update_plane (Daniel) -Release lock when failing to colorkey request with active scaler (Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: matthew.d.ro...@intel.com Reviewed-by: sonika.jin...@intel.com (v5) --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 266 +- drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c | 10 ++ 4 files changed, 219 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..22799fb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,204 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, -struct drm_framebuffer *fb, -int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF(pipe, 0), 0); - POSTING_READ(PLANE_CTL(pipe, 0)); - return; - } - - plane_ctl = PLANE_CTL_ENABLE | - PLANE_CTL_PIPE_GAMMA_ENABLE | - PLANE_CTL_PIPE_CSC_ENABLE; - - switch (fb-pixel_format) { + u32 plane_ctl_format = 0; + switch (pixel_format) { case DRM_FORMAT_RGB565: - plane_ctl |= PLANE_CTL_FORMAT_RGB_565; - break; - case DRM_FORMAT_XRGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - break; - case DRM_FORMAT_ARGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY; + plane_ctl_format = PLANE_CTL_FORMAT_RGB_565; break; case DRM_FORMAT_XBGR: - plane_ctl |= PLANE_CTL_ORDER_RGBX; - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; + plane_ctl_format = PLANE_CTL_FORMAT_XRGB_ | PLANE_CTL_ORDER_RGBX; break; + case DRM_FORMAT_XRGB: + plane_ctl_format = PLANE_CTL_FORMAT_XRGB_; + break; + /* +* XXX: For ARBG/ABGR formats we default to expecting scanout buffers +* to be already pre-multiplied. We need to add a knob (or a different +* DRM_FORMAT) for user-space to configure that. +*/ case
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
-Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Thursday, April 23, 2015 1:20 PM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: Matt Roper matthew.d.ro...@intel.com Testcase: kms_plane_scaling I wanted to pull this in but then spotted an issue. Since this needs one more round can you perhaps use the older version as a baseline again (without the refactoring)? Since skl planes aren't converted to universal planes yet it might be better to wait with refactoring until that's done actually. Two more comments below. Hi Daniel, Sorry for delay due to osts/f2f travel. Per last discussion at osts, you were ok to merge the changes made to skl update_plane functions to reduce its size. To undo these changes I have to redo all testing and also have to redo changes to upcoming nv12 changes. skl planes are already universal planes, Matt can comment more on this. But irrespective of whether skl planes universal or not with below changes, function sizes are more manageable. Regarding the FIXME and lock issue, will send out updated patch shortly. -Chandra ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: Matt Roper matthew.d.ro...@intel.com Testcase: kms_plane_scaling I wanted to pull this in but then spotted an issue. Since this needs one more round can you perhaps use the older version as a baseline again (without the refactoring)? Since skl planes aren't converted to universal planes yet it might be better to wait with refactoring until that's done actually. Two more comments below. -Daniel --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 261 +- drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c |9 ++ 4 files changed, 213 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..7f3ae8e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF(pipe, 0), 0); - POSTING_READ(PLANE_CTL(pipe, 0)); - return; - } - - plane_ctl = PLANE_CTL_ENABLE | - PLANE_CTL_PIPE_GAMMA_ENABLE | - PLANE_CTL_PIPE_CSC_ENABLE; - - switch (fb-pixel_format) { + u32 plane_ctl_format = 0; + switch (pixel_format) { case DRM_FORMAT_RGB565: - plane_ctl |= PLANE_CTL_FORMAT_RGB_565; - break; - case DRM_FORMAT_XRGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - break; - case DRM_FORMAT_ARGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY; + plane_ctl_format = PLANE_CTL_FORMAT_RGB_565; break; case DRM_FORMAT_XBGR: - plane_ctl |= PLANE_CTL_ORDER_RGBX; - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; + plane_ctl_format = PLANE_CTL_FORMAT_XRGB_ | PLANE_CTL_ORDER_RGBX; break; + case DRM_FORMAT_XRGB: + plane_ctl_format = PLANE_CTL_FORMAT_XRGB_; + break; + /* +
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. Please split out the refactoring into a prep patch since it's really hard to review for functional changes when the code moves around. But when that's split into a pure code-motion patch (which shouldn't have any functional changes) and a subsequent patch to actually change the logic then everything's a lot clearer. My BKM for splitting out prep patches is: 1. Create a new branch based on drm-intel-nightly. 2. Extract the first refactor step: You can either cherry-pick parts of the patch or create it anew. Don't bother thinking about conflicts. Test this patch to make sure you don't introduce a bisect issue. 3. Commit a revert of your refactor patch. 4. Cherry-pick the original patch, it should apply cleanly because the baseline is unchanged due to the revert. 5. You now have 3 patches in your branch - refactor patch - Revert refactor patch - real patch you want to split up Now do a rebase and squash together the Revert and the real patch. - First step is cleanly split out without ever having to deal with conflicts or risking bisectability issues. Your branch has now 2 patches: - refactor patch - real patch with refactor parts taken out Repeat until you've split up the patch into all the different parts. Cheers, Daniel Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: Matt Roper matthew.d.ro...@intel.com Testcase: kms_plane_scaling --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 261 +- drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c |9 ++ 4 files changed, 213 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..7f3ae8e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF(pipe, 0), 0); - POSTING_READ(PLANE_CTL(pipe, 0)); - return; - } - - plane_ctl = PLANE_CTL_ENABLE | - PLANE_CTL_PIPE_GAMMA_ENABLE | -
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
Sonika, I have rebased 13/14 and 14/14 of scaler series on top of 90/270 and did some refactoring to reduce function size. Previous versions are already reviewed and Matt gave r-b for those changes. Can you review v5 changes and give r-b or ack for them? You can see below (v5) list of changes triggered by 90/270. -Chandra -Original Message- From: Konduru, Chandra Sent: Wednesday, April 15, 2015 3:15 PM To: intel-gfx@lists.freedesktop.org Cc: Konduru, Chandra; Jindal, Sonika; Roper, Matthew D; Vetter, Daniel; Conselvan De Oliveira, Ander Subject: [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: Matt Roper matthew.d.ro...@intel.com Testcase: kms_plane_scaling --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 261 +-- --- drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c |9 ++ 4 files changed, 213 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..7f3ae8e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF(pipe, 0), 0); - POSTING_READ(PLANE_CTL(pipe, 0)); - return; - } - - plane_ctl = PLANE_CTL_ENABLE | - PLANE_CTL_PIPE_GAMMA_ENABLE | - PLANE_CTL_PIPE_CSC_ENABLE; - - switch (fb-pixel_format) { + u32 plane_ctl_format = 0; + switch (pixel_format) { case DRM_FORMAT_RGB565: - plane_ctl |= PLANE_CTL_FORMAT_RGB_565; - break; - case DRM_FORMAT_XRGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - break; - case DRM_FORMAT_ARGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY; + plane_ctl_format = PLANE_CTL_FORMAT_RGB_565; break; case DRM_FORMAT_XBGR: - plane_ctl |= PLANE_CTL_ORDER_RGBX; - plane_ctl |=
[Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) v5: -Rebased on top of 90/270 rotation changes (me) -Fixed an issue introduced by 90/270 changes where plane programming is using drm_plane-state rect instead of intel_plane-state rect. This change also required for scaling to work properly. (me) -With 90/270, updated limits to cover both portrait and landscape usages (me) -Refactored skylake_update_primary_plane to reduce its size (Daniel) Added helper functions for refactoring are comprehended enough to be used for skylake_update_plane (for sprite) too. One stop towards having single function for all planes. Signed-off-by: Chandra Konduru chandra.kond...@intel.com Reviewed-by: Matt Roper matthew.d.ro...@intel.com Testcase: kms_plane_scaling --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 261 +- drivers/gpu/drm/i915/intel_drv.h |8 +- drivers/gpu/drm/i915/intel_sprite.c |9 ++ 4 files changed, 213 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3c4b7cd..cb6d5f2 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -169,7 +169,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -178,6 +178,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f7a40f..7f3ae8e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2960,126 +2960,199 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) } } -static void skylake_update_primary_plane(struct drm_crtc *crtc, -struct drm_framebuffer *fb, -int x, int y) +u32 skl_plane_ctl_format(uint32_t pixel_format) { - struct drm_device *dev = crtc-dev; - struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - int pipe = intel_crtc-pipe; - u32 plane_ctl, stride_div, stride; - u32 tile_height, plane_offset, plane_size; - unsigned int rotation; - int x_offset, y_offset; - unsigned long surf_addr; - struct drm_plane *plane; - - if (!intel_crtc-primary_enabled) { - I915_WRITE(PLANE_CTL(pipe, 0), 0); - I915_WRITE(PLANE_SURF(pipe, 0), 0); - POSTING_READ(PLANE_CTL(pipe, 0)); - return; - } - - plane_ctl = PLANE_CTL_ENABLE | - PLANE_CTL_PIPE_GAMMA_ENABLE | - PLANE_CTL_PIPE_CSC_ENABLE; - - switch (fb-pixel_format) { + u32 plane_ctl_format = 0; + switch (pixel_format) { case DRM_FORMAT_RGB565: - plane_ctl |= PLANE_CTL_FORMAT_RGB_565; - break; - case DRM_FORMAT_XRGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - break; - case DRM_FORMAT_ARGB: - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - plane_ctl |= PLANE_CTL_ALPHA_SW_PREMULTIPLY; + plane_ctl_format = PLANE_CTL_FORMAT_RGB_565; break; case DRM_FORMAT_XBGR: - plane_ctl |= PLANE_CTL_ORDER_RGBX; - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; + plane_ctl_format = PLANE_CTL_FORMAT_XRGB_ | PLANE_CTL_ORDER_RGBX; break; + case DRM_FORMAT_XRGB: + plane_ctl_format = PLANE_CTL_FORMAT_XRGB_; + break; + /* +* XXX: For ARBG/ABGR formats we default to expecting scanout buffers +* to be already pre-multiplied. We need to add a knob (or a different +* DRM_FORMAT) for user-space to configure that. +*/ case DRM_FORMAT_ABGR: - plane_ctl |= PLANE_CTL_ORDER_RGBX; - plane_ctl |= PLANE_CTL_FORMAT_XRGB_; - plane_ctl |=
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Tue, Apr 07, 2015 at 03:28:46PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 96 -- drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_sprite.c |9 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 2fc04ec..8f759c6 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -167,7 +167,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -176,6 +176,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index aa4da1f..c7ee232 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, And one real piece of review feedback: This function is now definitely too long. Same holds for the sprite update function below. Can you (or Sonika) please follow up with a few patches to extract subroutines to make this a bit easier to read? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Tue, Apr 07, 2015 at 03:28:46PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com Ok there's a real functional conflict here now with 90/270 rotation. It's big enough that imo we also need to add igt coverage to test rotation+scaling together (the scaled plane setup is different from the normal one, and we need to also make sure we scale correctly itself when rotated). Chandra can you please figure out with Sonika who can do this rebasing/igt extension? Thanks, Daniel --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 96 -- drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_sprite.c |9 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 2fc04ec..8f759c6 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -167,7 +167,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -176,6 +176,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index aa4da1f..c7ee232 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, int pipe = intel_crtc-pipe; u32 plane_ctl, stride_div; unsigned long surf_addr; + struct intel_crtc_state *crtc_state = intel_crtc-config; + struct intel_plane_state *plane_state; + int src_x = 0, src_y = 0, src_w = 0, src_h = 0; + int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; + int scaler_id = -1; + + plane_state = crtc-primary ? + to_intel_plane_state(crtc-primary-state) : NULL; if (!intel_crtc-primary_enabled) { I915_WRITE(PLANE_CTL(pipe, 0), 0); @@ -3046,12 +3054,40 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, fb-pixel_format); surf_addr = intel_plane_obj_offset(to_intel_plane(crtc-primary), obj); + if (plane_state) { + scaler_id = plane_state-scaler_id; + src_x = plane_state-src.x1 16; + src_y = plane_state-src.y1 16; + src_w = drm_rect_width(plane_state-src) 16; + src_h = drm_rect_height(plane_state-src) 16; + dst_x = plane_state-dst.x1; + dst_y = plane_state-dst.y1; + dst_w = drm_rect_width(plane_state-dst); + dst_h = drm_rect_height(plane_state-dst); + } + I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); I915_WRITE(PLANE_POS(pipe, 0), 0); + + if (src_w src_h dst_w dst_h scaler_id = 0) { + uint32_t ps_ctrl = 0; + + WARN_ON(x != src_x || y != src_y); + ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) | + crtc_state-scaler_state.scalers[scaler_id].mode; + I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); + I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0); + I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x 16) | dst_y); + I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w 16) | dst_h); + + I915_WRITE(PLANE_SIZE(pipe, 0), ((src_h - 1) 16) | (src_w - 1)); + } else { + I915_WRITE(PLANE_SIZE(pipe, 0), + (intel_crtc-config-pipe_src_h - 1) 16 | + (intel_crtc-config-pipe_src_w - 1)); + } + I915_WRITE(PLANE_OFFSET(pipe, 0), (y 16) | x); - I915_WRITE(PLANE_SIZE(pipe, 0), -(intel_crtc-config-pipe_src_h - 1) 16 | -(intel_crtc-config-pipe_src_w - 1)); I915_WRITE(PLANE_STRIDE(pipe, 0), fb-pitches[0] / stride_div); I915_WRITE(PLANE_SURF(pipe, 0), surf_addr); @@ -12778,6 +12814,36 @@
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
-Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Monday, April 13, 2015 11:13 AM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers On Tue, Apr 07, 2015 at 03:28:46PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com Ok there's a real functional conflict here now with 90/270 rotation. It's big enough that imo we also need to add igt coverage to test rotation+scaling together (the scaled plane setup is different from the normal one, and we need to also make sure we scale correctly itself when rotated). Chandra can you please figure out with Sonika who can do this rebasing/igt extension? To give some context, this was one of the reasons I gave heads up to Sonika to rebase 90/270 on top of scalers but looks they weren't. Agree that we need igt for rotation+scaling too. In current kms_plane_scaling didn't planned to cover 90/270 for the same reason that igt for 90/270 taking care of that. Sure, will sync up offline with Sonika and get back on the last two patches and igt too. Thanks, Daniel --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 96 -- drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_sprite.c |9 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 2fc04ec..8f759c6 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -167,7 +167,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -176,6 +176,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index aa4da1f..c7ee232 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, int pipe = intel_crtc-pipe; u32 plane_ctl, stride_div; unsigned long surf_addr; + struct intel_crtc_state *crtc_state = intel_crtc-config; + struct intel_plane_state *plane_state; + int src_x = 0, src_y = 0, src_w = 0, src_h = 0; + int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; + int scaler_id = -1; + + plane_state = crtc-primary ? + to_intel_plane_state(crtc-primary-state) : NULL; if (!intel_crtc-primary_enabled) { I915_WRITE(PLANE_CTL(pipe, 0), 0); @@ -3046,12 +3054,40 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, fb-pixel_format); surf_addr = intel_plane_obj_offset(to_intel_plane(crtc-primary), obj); + if (plane_state) { + scaler_id = plane_state-scaler_id; + src_x = plane_state-src.x1 16; + src_y = plane_state-src.y1 16; + src_w = drm_rect_width(plane_state-src) 16; + src_h = drm_rect_height(plane_state-src) 16; + dst_x = plane_state-dst.x1; + dst_y = plane_state-dst.y1; + dst_w = drm_rect_width(plane_state-dst); + dst_h = drm_rect_height(plane_state-dst); + } + I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); I915_WRITE(PLANE_POS(pipe, 0), 0); + + if (src_w src_h dst_w dst_h scaler_id = 0) { + uint32_t ps_ctrl = 0; + + WARN_ON(x != src_x || y != src_y); + ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) | + crtc_state-scaler_state.scalers[scaler_id].mode; + I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); + I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0); + I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id
Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
On Thu, Apr 09, 2015 at 10:14:36PM +, Konduru, Chandra wrote: -Original Message- From: Roper, Matthew D Sent: Thursday, April 09, 2015 2:52 PM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander Subject: Re: [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers On Tue, Apr 07, 2015 at 03:28:46PM -0700, Chandra Konduru wrote: This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) See comments on patch #6 that relate to this. If you want to take the approach here (perform the unshift in skl_update_plane) then you also need to do the same for all other platforms (ivb, ilk, vlv, etc.). But my suggestion on the other patch (do the unshifting in commit_plane and leave skl_update_plane alone) might be a bit simpler. Above v4: comment is saying that the change done in v3 was undone to keep primary_plane src rect in 16.16 format. As I responded to your patch #6 comment, moving unshift hunk (which is for sprite plane) from #14 to #6 will avoid any potential bisect issue that you mentioned. For other than skylake, primary planes platform level update functions doesn't use src_rect instead operate based on passed parameters which are in regular ints. Only for skylake primary plane update function, src rect is used for windowing, scaling purposes. I merged up to patch 12, but this one here doesn't apply any more and I'm not sure whether there's any changes still needed to it (it sounds like not, but chasing that unshifting business is tricky). Chandra, can you please resend rebased patches (with Matt's r-b added)? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
This patch enables skylake primary plane scaling using shared scalers atomic desgin. v2: -use single copy of scaler limits (Matt) v3: -move detach_scalers to crtc commit path (Matt) -use values in plane_state-src as regular integers (me) v4: -changes to align with updated scaler structures (Matt, me) -keep plane src rect in 16.16 format (Matt, Daniel) Signed-off-by: Chandra Konduru chandra.kond...@intel.com --- drivers/gpu/drm/i915/intel_atomic.c |5 +- drivers/gpu/drm/i915/intel_display.c | 96 -- drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_sprite.c |9 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 2fc04ec..8f759c6 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -167,7 +167,7 @@ int intel_atomic_commit(struct drm_device *dev, plane-state-state = NULL; } - /* swap crtc_state */ + /* swap crtc_scaler_state */ for (i = 0; i dev-mode_config.num_crtc; i++) { struct drm_crtc *crtc = state-crtcs[i]; if (!crtc) { @@ -176,6 +176,9 @@ int intel_atomic_commit(struct drm_device *dev, to_intel_crtc(crtc)-config-scaler_state = to_intel_crtc_state(state-crtc_states[i])-scaler_state; + + if (INTEL_INFO(dev)-gen = 9) + skl_detach_scalers(to_intel_crtc(crtc)); } drm_atomic_helper_commit_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index aa4da1f..c7ee232 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2978,6 +2978,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, int pipe = intel_crtc-pipe; u32 plane_ctl, stride_div; unsigned long surf_addr; + struct intel_crtc_state *crtc_state = intel_crtc-config; + struct intel_plane_state *plane_state; + int src_x = 0, src_y = 0, src_w = 0, src_h = 0; + int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; + int scaler_id = -1; + + plane_state = crtc-primary ? + to_intel_plane_state(crtc-primary-state) : NULL; if (!intel_crtc-primary_enabled) { I915_WRITE(PLANE_CTL(pipe, 0), 0); @@ -3046,12 +3054,40 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, fb-pixel_format); surf_addr = intel_plane_obj_offset(to_intel_plane(crtc-primary), obj); + if (plane_state) { + scaler_id = plane_state-scaler_id; + src_x = plane_state-src.x1 16; + src_y = plane_state-src.y1 16; + src_w = drm_rect_width(plane_state-src) 16; + src_h = drm_rect_height(plane_state-src) 16; + dst_x = plane_state-dst.x1; + dst_y = plane_state-dst.y1; + dst_w = drm_rect_width(plane_state-dst); + dst_h = drm_rect_height(plane_state-dst); + } + I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); I915_WRITE(PLANE_POS(pipe, 0), 0); + + if (src_w src_h dst_w dst_h scaler_id = 0) { + uint32_t ps_ctrl = 0; + + WARN_ON(x != src_x || y != src_y); + ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) | + crtc_state-scaler_state.scalers[scaler_id].mode; + I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); + I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0); + I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x 16) | dst_y); + I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w 16) | dst_h); + + I915_WRITE(PLANE_SIZE(pipe, 0), ((src_h - 1) 16) | (src_w - 1)); + } else { + I915_WRITE(PLANE_SIZE(pipe, 0), + (intel_crtc-config-pipe_src_h - 1) 16 | + (intel_crtc-config-pipe_src_w - 1)); + } + I915_WRITE(PLANE_OFFSET(pipe, 0), (y 16) | x); - I915_WRITE(PLANE_SIZE(pipe, 0), - (intel_crtc-config-pipe_src_h - 1) 16 | - (intel_crtc-config-pipe_src_w - 1)); I915_WRITE(PLANE_STRIDE(pipe, 0), fb-pitches[0] / stride_div); I915_WRITE(PLANE_SURF(pipe, 0), surf_addr); @@ -12778,6 +12814,36 @@ intel_cleanup_plane_fb(struct drm_plane *plane, } } +int +skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state) +{ + int max_scale; + struct drm_device *dev; + struct drm_i915_private *dev_priv; + int crtc_clock, cdclk; + + if (!intel_crtc || !crtc_state) + return DRM_PLANE_HELPER_NO_SCALING; + + dev = intel_crtc-base.dev; + dev_priv = dev-dev_private; +