From patchwork Fri Sep 18 01:55:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Roper X-Patchwork-Id: 7212371 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B38B9BEEC1 for ; Fri, 18 Sep 2015 01:55:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6AFB1206FF for ; Fri, 18 Sep 2015 01:55:56 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 953F6207A8 for ; Fri, 18 Sep 2015 01:55:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A60B36E27F; Thu, 17 Sep 2015 18:55:34 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id BD8E36E067 for ; Thu, 17 Sep 2015 18:55:33 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 17 Sep 2015 18:55:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,550,1437462000"; d="scan'208";a="792174525" Received: from mdroper-hswdev.fm.intel.com (HELO mdroper-hswdev) ([10.1.134.215]) by fmsmga001.fm.intel.com with ESMTP; 17 Sep 2015 18:55:28 -0700 Received: from mattrope by mdroper-hswdev with local (Exim 4.84) (envelope-from ) id 1Zcktc-0006Ci-B9; Thu, 17 Sep 2015 18:55:28 -0700 From: Matt Roper To: intel-gfx@lists.freedesktop.org Date: Thu, 17 Sep 2015 18:55:21 -0700 Message-Id: <1442541321-23813-1-git-send-email-matthew.d.roper@intel.com> X-Mailer: git-send-email 2.1.4 Subject: [Intel-gfx] [PATCH] drm/i915: Clean up intel_plane->plane usage X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP intel_plane->plane has inconsistent meanings across the driver: * For primary planes, intel_plane->plane is usually the pipe number (except for old platforms where it had to be swapped for FBC reasons). * For sprite planes, intel_plane->plane represents the sprite index for the specific CRTC the sprite belongs to (0, 1, 2, ...) * For cursor planes, intel_plane->plane is again the pipe number, but without the FBC swapping on old platforms. This overloading of the field can be quite confusing and easily leads to bugs due to differences in how the hardware actually gets programmed (e.g., SKL code needs to use p->plane + 1 because the hardware expects universal plane ID's that include the primary, whereas VLV code needs to use just p->plane because hardware expects a sprite index that does not include the primary). Let's try to clean up this situation by giving intel_plane->plane a consistent meaning across all plane types, and then update all uses across driver code to use macros to interpret it properly. The following macros should be used in the code where we previously accessed p->plane and will do additional plane type checking to help prevent misuse: * I915_UNIVERSAL_NUM(p) --- Universal plane ID (primary = 0, sprites start from 1); intended for use in gen9+ code. * I915_I915_PRIMARY_NUM(p) --- Primary plane ID for pre-gen9 platforms; determines whether FBC-related swapping is needed or not. * I915_SPRITE_NUM(p) --- Sprite plane ID (first sprite = 0); intended for use in pre-gen9 code. Cc: Maarten Lankhorst Signed-off-by: Matt Roper Reviewed-by: Maarten Lankhorst --- drivers/gpu/drm/i915/i915_drv.h | 24 ++++++++++++++++++------ drivers/gpu/drm/i915/intel_display.c | 12 ++++-------- drivers/gpu/drm/i915/intel_pm.c | 10 +++++----- drivers/gpu/drm/i915/intel_sprite.c | 13 +++++++------ 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3bf8a9b..132fe53 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -131,22 +131,34 @@ enum transcoder { #define transcoder_name(t) ((t) + 'A') /* - * This is the maximum (across all platforms) number of planes (primary + - * sprites) that can be active at the same time on one pipe. - * - * This value doesn't count the cursor plane. + * I915_MAX_PLANES in the enum below is the maximum (across all platforms) + * number of planes per CRTC. Not all platforms really have this many planes, + * which means some arrays of size I915_MAX_PLANES may have unused entries + * between the topmost sprite plane and the cursor plane. */ -#define I915_MAX_PLANES 4 - enum plane { PLANE_A = 0, + PLANE_PRIMARY = PLANE_A, PLANE_B, PLANE_C, + PLANE_CURSOR, + I915_MAX_PLANES, }; #define plane_name(p) ((p) + 'A') #define sprite_name(p, s) ((p) * INTEL_INFO(dev)->num_sprites[(p)] + (s) + 'A') +#define I915_UNIVERSAL_NUM(p) ( \ + WARN_ON(p->base.type == DRM_PLANE_TYPE_CURSOR), \ + p->plane) +#define I915_PRIMARY_NUM(p) ( \ + WARN_ON(p->base.type != DRM_PLANE_TYPE_PRIMARY), \ + (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) ? \ + !p->pipe : p->pipe) +#define I915_SPRITE_NUM(p) ( \ + WARN_ON(p->base.type != DRM_PLANE_TYPE_OVERLAY), \ + p->plane - 1) + enum port { PORT_A = 0, PORT_B, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index fc00867..dab0b71 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11976,16 +11976,14 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d " "disabled, scaler_id = %d\n", plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD", - plane->base.id, intel_plane->pipe, - (crtc->base.primary == plane) ? 0 : intel_plane->plane + 1, + plane->base.id, intel_plane->pipe, intel_plane->plane, drm_plane_index(plane), state->scaler_id); continue; } DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d enabled", plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD", - plane->base.id, intel_plane->pipe, - crtc->base.primary == plane ? 0 : intel_plane->plane + 1, + plane->base.id, intel_plane->pipe, intel_plane->plane, drm_plane_index(plane)); DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x", fb->base.id, fb->width, fb->height, fb->pixel_format); @@ -13547,13 +13545,11 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, state->scaler_id = -1; } primary->pipe = pipe; - primary->plane = pipe; + primary->plane = 0; primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe); primary->check_plane = intel_check_primary_plane; primary->commit_plane = intel_commit_primary_plane; primary->disable_plane = intel_disable_primary_plane; - if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) - primary->plane = !pipe; if (INTEL_INFO(dev)->gen >= 9) { intel_primary_formats = skl_primary_formats; @@ -13699,7 +13695,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, cursor->can_scale = false; cursor->max_downscale = 1; cursor->pipe = pipe; - cursor->plane = pipe; + cursor->plane = PLANE_CURSOR; cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe); cursor->check_plane = intel_check_cursor_plane; cursor->commit_plane = intel_commit_cursor_plane; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 62de97e..9db19f2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1131,7 +1131,7 @@ static void vlv_invert_wms(struct intel_crtc *crtc) wm_state->wm[level].primary; break; case DRM_PLANE_TYPE_OVERLAY: - sprite = plane->plane; + sprite = I915_SPRITE_NUM(plane); wm_state->wm[level].sprite[sprite] = plane->wm.fifo_size - wm_state->wm[level].sprite[sprite]; break; @@ -1195,7 +1195,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc) wm_state->wm[level].primary = wm; break; case DRM_PLANE_TYPE_OVERLAY: - sprite = plane->plane; + sprite = I915_SPRITE_NUM(plane); wm_state->wm[level].sprite[sprite] = wm; break; } @@ -1221,7 +1221,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc) wm_state->wm[level].primary); break; case DRM_PLANE_TYPE_OVERLAY: - sprite = plane->plane; + sprite = I915_SPRITE_NUM(plane); for (level = 0; level < wm_state->num_levels; level++) wm_state->sr[level].plane = min(wm_state->sr[level].plane, @@ -1257,7 +1257,7 @@ static void vlv_pipe_set_fifo_size(struct intel_crtc *crtc) if (plane->base.type == DRM_PLANE_TYPE_PRIMARY) sprite0_start = plane->wm.fifo_size; - else if (plane->plane == 0) + else if (I915_SPRITE_NUM(plane) == 0) sprite1_start = sprite0_start + plane->wm.fifo_size; else fifo_size = sprite1_start + plane->wm.fifo_size; @@ -4076,7 +4076,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev) plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, 0); break; case DRM_PLANE_TYPE_OVERLAY: - sprite = plane->plane; + sprite = I915_SPRITE_NUM(plane); plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, sprite + 1); break; } diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 4d27243..11ca40a 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -180,7 +180,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, struct intel_plane *intel_plane = to_intel_plane(drm_plane); struct drm_i915_gem_object *obj = intel_fb_obj(fb); const int pipe = intel_plane->pipe; - const int plane = intel_plane->plane + 1; + const int plane = I915_UNIVERSAL_NUM(intel_plane); u32 plane_ctl, stride_div, stride; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); const struct drm_intel_sprite_colorkey *key = @@ -280,7 +280,7 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(dplane); const int pipe = intel_plane->pipe; - const int plane = intel_plane->plane + 1; + const int plane = I915_UNIVERSAL_NUM(intel_plane); I915_WRITE(PLANE_CTL(pipe, plane), 0); @@ -294,7 +294,7 @@ static void chv_update_csc(struct intel_plane *intel_plane, uint32_t format) { struct drm_i915_private *dev_priv = intel_plane->base.dev->dev_private; - int plane = intel_plane->plane; + int plane = I915_SPRITE_NUM(intel_plane); /* Seems RGB data bypasses the CSC always */ if (!format_is_yuv(format)) @@ -342,7 +342,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, struct intel_plane *intel_plane = to_intel_plane(dplane); struct drm_i915_gem_object *obj = intel_fb_obj(fb); int pipe = intel_plane->pipe; - int plane = intel_plane->plane; + int plane = I915_SPRITE_NUM(intel_plane); u32 sprctl; unsigned long sprsurf_offset, linear_offset; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); @@ -461,7 +461,7 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(dplane); int pipe = intel_plane->pipe; - int plane = intel_plane->plane; + int plane = I915_SPRITE_NUM(intel_plane); I915_WRITE(SPCNTR(pipe, plane), 0); @@ -1121,8 +1121,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) return -ENODEV; } + /* primary = 0, sprites start from 1 */ + intel_plane->plane = plane + 1; intel_plane->pipe = pipe; - intel_plane->plane = plane; intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane); intel_plane->check_plane = intel_check_sprite_plane; intel_plane->commit_plane = intel_commit_sprite_plane;