Re: [Intel-gfx] [PATCH 09/20] drm/i915: Add helper function to update scaler_users in crtc_state
On Wed, Apr 01, 2015 at 07:59:38PM -0700, Chandra Konduru wrote:
This helper function stages a scaler request for a plane/crtc into
crtc_state-scaler_users (which is a bit field). It also performs
required checks before staging any change into scaler_state.
v2:
-updates to use single copy of scaler limits (Matt)
-added force detach parameter for pfit disable purpose (me)
v3:
-updated function header to kerneldoc format (Matt)
-dropped need_scaling checks (Matt)
v4:
-move clearing of scaler id from commit path to check path (Matt)
-updated colorkey checks based on recent updates (me)
-squashed scaler check while enabling colorkey to here (me)
-use values in plane_state-src as regular integers (me)
Signed-off-by: Chandra Konduru chandra.kond...@intel.com
As noted in the previous review cycle, this should at least be squashed
with patch 17, which is when you start calling this.
---
drivers/gpu/drm/i915/intel_display.c | 144
++
drivers/gpu/drm/i915/intel_drv.h |3 +
drivers/gpu/drm/i915/intel_sprite.c |9 +++
3 files changed, 156 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8b2eff4..603a2dc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12594,6 +12594,150 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
}
}
+/**
+ * skl_update_scaler_users - Stages update to crtc's scaler state
+ * @intel_crtc: crtc
+ * @crtc_state: crtc_state
+ * @plane: plane (NULL indicates crtc is requesting update)
+ * @plane_state: plane's state
+ * @force_detach: request unconditional detachment of scaler
+ *
+ * This function updates scaler state for requested plane or crtc.
+ * To request scaler usage update for a plane, caller shall pass plane
pointer.
+ * To request scaler usage update for crtc, caller shall pass plane pointer
+ * as NULL.
+ *
+ * Return
+ * 0 - scaler_usage updated successfully
+ *error - requested scaling cannot be supported or other error condition
+ */
+int
+skl_update_scaler_users(
+ struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state,
+ struct intel_plane *intel_plane, struct intel_plane_state *plane_state,
+ int force_detach)
+{
+ int need_scaling;
+ int idx;
+ int src_w, src_h, dst_w, dst_h;
+ int *scaler_id;
+ struct drm_framebuffer *fb;
+ struct intel_crtc_scaler_state *scaler_state;
+
+ if (!intel_crtc || !crtc_state ||
+ (intel_plane intel_plane-base.type ==
DRM_PLANE_TYPE_CURSOR))
It doesn't look possible to get here with a cursor plane to me. Maybe
wrap that plane type test in a WARN_ON() so that we notice if we screw
up?
+ return 0;
+
+ scaler_state = crtc_state-scaler_state;
+
+ if (!scaler_state-num_scalers) {
+ DRM_DEBUG_KMS(crtc_state = %p, num_scalers = %d\n, crtc_state,
+ scaler_state-num_scalers);
+ return 0;
+ }
+
+ idx = intel_plane ? drm_plane_index(intel_plane-base) :
SKL_CRTC_INDEX;
+ fb = intel_plane ? plane_state-base.fb : NULL;
+
+ if (intel_plane) {
+ src_w = drm_rect_width(plane_state-src);
+ src_h = drm_rect_height(plane_state-src);
+ dst_w = drm_rect_width(plane_state-dst);
+ dst_h = drm_rect_height(plane_state-dst);
+ scaler_id = plane_state-scaler_id;
+ } else {
+ struct drm_display_mode *adjusted_mode =
+ crtc_state-base.adjusted_mode;
+ src_w = crtc_state-pipe_src_w;
+ src_h = crtc_state-pipe_src_h;
+ dst_w = adjusted_mode-hdisplay;
+ dst_h = adjusted_mode-vdisplay;
+ scaler_id = scaler_state-scaler_id;
+ }
+ need_scaling = (src_w != dst_w || src_h != dst_h);
+
+ /*
+ * if plane is being disabled or scaler is no more required or force
detach
+ * - free scaler binded to this plane/crtc
+ * - in order to do this, update crtc-scaler_usage
+ *
+ * Here scaler state in crtc_state is set free so that
+ * scaler can be assigned to other user. Actual register
+ * update to free the scaler is done in plane/panel-fit programming.
+ * For this purpose crtc/plane_state-scaler_id isn't reset here.
+ */
+ if (force_detach || !need_scaling || (intel_plane
I guess this force_detach approach is a bit of a necessary evil because
we haven't converted to atomic for full modesets yet (or have real
crtc_state tracking yet). I'm a little bit worried that it will make
the eventual atomic conversion a bit trickier, but I guess there isn't
much we can do about that if we're planning to merge this series on
today's driver rather than waiting to finish atomic conversion.
Ander is Cc'd on this, and it intersects mainly with his ongoing
[Intel-gfx] [PATCH 09/20] drm/i915: Add helper function to update scaler_users in crtc_state
This helper function stages a scaler request for a plane/crtc into
crtc_state-scaler_users (which is a bit field). It also performs
required checks before staging any change into scaler_state.
v2:
-updates to use single copy of scaler limits (Matt)
-added force detach parameter for pfit disable purpose (me)
v3:
-updated function header to kerneldoc format (Matt)
-dropped need_scaling checks (Matt)
v4:
-move clearing of scaler id from commit path to check path (Matt)
-updated colorkey checks based on recent updates (me)
-squashed scaler check while enabling colorkey to here (me)
-use values in plane_state-src as regular integers (me)
Signed-off-by: Chandra Konduru chandra.kond...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 144 ++
drivers/gpu/drm/i915/intel_drv.h |3 +
drivers/gpu/drm/i915/intel_sprite.c |9 +++
3 files changed, 156 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8b2eff4..603a2dc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12594,6 +12594,150 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
}
}
+/**
+ * skl_update_scaler_users - Stages update to crtc's scaler state
+ * @intel_crtc: crtc
+ * @crtc_state: crtc_state
+ * @plane: plane (NULL indicates crtc is requesting update)
+ * @plane_state: plane's state
+ * @force_detach: request unconditional detachment of scaler
+ *
+ * This function updates scaler state for requested plane or crtc.
+ * To request scaler usage update for a plane, caller shall pass plane pointer.
+ * To request scaler usage update for crtc, caller shall pass plane pointer
+ * as NULL.
+ *
+ * Return
+ * 0 - scaler_usage updated successfully
+ *error - requested scaling cannot be supported or other error condition
+ */
+int
+skl_update_scaler_users(
+ struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state,
+ struct intel_plane *intel_plane, struct intel_plane_state *plane_state,
+ int force_detach)
+{
+ int need_scaling;
+ int idx;
+ int src_w, src_h, dst_w, dst_h;
+ int *scaler_id;
+ struct drm_framebuffer *fb;
+ struct intel_crtc_scaler_state *scaler_state;
+
+ if (!intel_crtc || !crtc_state ||
+ (intel_plane intel_plane-base.type ==
DRM_PLANE_TYPE_CURSOR))
+ return 0;
+
+ scaler_state = crtc_state-scaler_state;
+
+ if (!scaler_state-num_scalers) {
+ DRM_DEBUG_KMS(crtc_state = %p, num_scalers = %d\n, crtc_state,
+ scaler_state-num_scalers);
+ return 0;
+ }
+
+ idx = intel_plane ? drm_plane_index(intel_plane-base) :
SKL_CRTC_INDEX;
+ fb = intel_plane ? plane_state-base.fb : NULL;
+
+ if (intel_plane) {
+ src_w = drm_rect_width(plane_state-src);
+ src_h = drm_rect_height(plane_state-src);
+ dst_w = drm_rect_width(plane_state-dst);
+ dst_h = drm_rect_height(plane_state-dst);
+ scaler_id = plane_state-scaler_id;
+ } else {
+ struct drm_display_mode *adjusted_mode =
+ crtc_state-base.adjusted_mode;
+ src_w = crtc_state-pipe_src_w;
+ src_h = crtc_state-pipe_src_h;
+ dst_w = adjusted_mode-hdisplay;
+ dst_h = adjusted_mode-vdisplay;
+ scaler_id = scaler_state-scaler_id;
+ }
+ need_scaling = (src_w != dst_w || src_h != dst_h);
+
+ /*
+* if plane is being disabled or scaler is no more required or force
detach
+* - free scaler binded to this plane/crtc
+* - in order to do this, update crtc-scaler_usage
+*
+* Here scaler state in crtc_state is set free so that
+* scaler can be assigned to other user. Actual register
+* update to free the scaler is done in plane/panel-fit programming.
+* For this purpose crtc/plane_state-scaler_id isn't reset here.
+*/
+ if (force_detach || !need_scaling || (intel_plane
+ (!fb || !plane_state-visible))) {
+ if (*scaler_id = 0) {
+ scaler_state-scaler_users = ~(1 idx);
+ scaler_state-scalers[*scaler_id].in_use = 0;
+ *scaler_id = -1;
+
+ DRM_DEBUG_KMS(Staged freeing scaler id %u.%u from
%s:%d
+ crtc_state = %p scaler_users = 0x%x\n,
+ intel_crtc-pipe, *scaler_id, intel_plane ?
PLANE : CRTC,
+ intel_plane ? intel_plane-base.base.id :
+ intel_crtc-base.base.id, crtc_state,
+ scaler_state-scaler_users);
+ }
+ return 0;
+ }
+
+ /* range checks */
+ if (src_w scaler_state-min_src_w ||
