From patchwork Wed Jul 12 10:43:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 9836483 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 642D1602BD for ; Wed, 12 Jul 2017 10:43:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 54F43285F9 for ; Wed, 12 Jul 2017 10:43:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 498B1285FD; Wed, 12 Jul 2017 10:43:46 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 16FB32860E for ; Wed, 12 Jul 2017 10:43:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 760316E3FF; Wed, 12 Jul 2017 10:43:44 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4E87E6E3FF; Wed, 12 Jul 2017 10:43:43 +0000 (UTC) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Jul 2017 03:43:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,349,1496127600"; d="scan'208";a="285916621" Received: from fbrandid-mobl.ger.corp.intel.com (HELO patser.lan) ([10.252.51.215]) by fmsmga004.fm.intel.com with ESMTP; 12 Jul 2017 03:43:41 -0700 To: Daniel Vetter References: <20170712081344.25495-1-maarten.lankhorst@linux.intel.com> <20170712081344.25495-8-maarten.lankhorst@linux.intel.com> <20170712091540.rnjldq4bbdiqrcur@phenom.ffwll.local> From: Maarten Lankhorst Message-ID: <485c9b80-43da-a8d0-6379-3620fddd8334@linux.intel.com> Date: Wed, 12 Jul 2017 12:43:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170712091540.rnjldq4bbdiqrcur@phenom.ffwll.local> Content-Language: en-US Cc: linux-renesas-soc@vger.kernel.org, intel-gfx@lists.freedesktop.org, Laurent Pinchart , dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v2 07/16] drm/rcar-du: Use new iterator macros, v2. 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Op 12-07-17 om 11:15 schreef Daniel Vetter: > On Wed, Jul 12, 2017 at 10:13:35AM +0200, Maarten Lankhorst wrote: >> for_each_obj_in_state is about to be removed, so use the correct new >> iterator macros. >> >> Signed-off-by: Maarten Lankhorst >> Cc: Laurent Pinchart >> Cc: linux-renesas-soc@vger.kernel.org > Looks correct, but I think Laurent has a patch to rework/remove this. > Found something below anyway. > >> --- >> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 41 ++++++++++++++++----------------- >> 1 file changed, 20 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c >> index dcde6288da6c..dfd84e3c8c25 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c >> @@ -51,12 +51,9 @@ >> */ >> >> static bool rcar_du_plane_needs_realloc(struct rcar_du_plane *plane, >> + const struct rcar_du_plane_state *cur_state, >> struct rcar_du_plane_state *new_state) >> { >> - struct rcar_du_plane_state *cur_state; >> - >> - cur_state = to_rcar_plane_state(plane->plane.state); >> - >> /* Lowering the number of planes doesn't strictly require reallocation >> * as the extra hardware plane will be freed when committing, but doing >> * so could lead to more fragmentation. >> @@ -141,16 +138,17 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, >> unsigned int groups = 0; >> unsigned int i; >> struct drm_plane *drm_plane; >> - struct drm_plane_state *drm_plane_state; >> + struct drm_plane_state *old_drm_plane_state, *new_drm_plane_state; >> >> /* Check if hardware planes need to be reallocated. */ >> - for_each_plane_in_state(state, drm_plane, drm_plane_state, i) { >> - struct rcar_du_plane_state *plane_state; >> + for_each_oldnew_plane_in_state(state, drm_plane, old_drm_plane_state, new_drm_plane_state, i) { >> + struct rcar_du_plane_state *old_plane_state, *new_plane_state; >> struct rcar_du_plane *plane; >> unsigned int index; >> >> plane = to_rcar_plane(drm_plane); >> - plane_state = to_rcar_plane_state(drm_plane_state); >> + old_plane_state = to_rcar_plane_state(old_drm_plane_state); >> + new_plane_state = to_rcar_plane_state(new_drm_plane_state); >> >> dev_dbg(rcdu->dev, "%s: checking plane (%u,%tu)\n", __func__, >> plane->group->index, plane - plane->group->planes); >> @@ -159,19 +157,19 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, >> * the full reallocation procedure. Just mark the hardware >> * plane(s) as freed. >> */ >> - if (!plane_state->format) { >> + if (!new_plane_state->format) { >> dev_dbg(rcdu->dev, "%s: plane is being disabled\n", >> __func__); >> index = plane - plane->group->planes; >> group_freed_planes[plane->group->index] |= 1 << index; >> - plane_state->hwindex = -1; >> + new_plane_state->hwindex = -1; >> continue; >> } >> >> /* If the plane needs to be reallocated mark it as such, and >> * mark the hardware plane(s) as free. >> */ >> - if (rcar_du_plane_needs_realloc(plane, plane_state)) { >> + if (rcar_du_plane_needs_realloc(plane, old_plane_state, new_plane_state)) { >> dev_dbg(rcdu->dev, "%s: plane needs reallocation\n", >> __func__); >> groups |= 1 << plane->group->index; >> @@ -179,7 +177,7 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, >> >> index = plane - plane->group->planes; >> group_freed_planes[plane->group->index] |= 1 << index; >> - plane_state->hwindex = -1; >> + new_plane_state->hwindex = -1; >> } >> } >> >> @@ -226,16 +224,16 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, >> continue; >> } >> >> - plane_state = to_rcar_plane_state(plane->plane.state); >> + new_plane_state = to_rcar_plane_state(plane->plane.state); >> used_planes |= rcar_du_plane_hwmask(plane_state); >> >> dev_dbg(rcdu->dev, >> "%s: plane (%u,%tu) uses %u hwplanes (index %d)\n", >> __func__, plane->group->index, >> plane - plane->group->planes, >> - plane_state->format ? >> - plane_state->format->planes : 0, >> - plane_state->hwindex); >> + new_plane_state->format ? >> + new_plane_state->format->planes : 0, >> + new_plane_state->hwindex); >> } >> >> group_free_planes[index] = 0xff & ~used_planes; >> @@ -246,15 +244,16 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, >> } >> >> /* Reallocate hardware planes for each plane that needs it. */ >> - for_each_plane_in_state(state, drm_plane, drm_plane_state, i) { >> - struct rcar_du_plane_state *plane_state; >> + for_each_oldnew_plane_in_state(state, drm_plane, old_drm_plane_state, new_drm_plane_state, i) { >> + struct rcar_du_plane_state *old_plane_state, *new_plane_state; >> struct rcar_du_plane *plane; >> unsigned int crtc_planes; >> unsigned int free; >> int idx; >> >> plane = to_rcar_plane(drm_plane); >> - plane_state = to_rcar_plane_state(drm_plane_state); >> + old_plane_state = to_rcar_plane_state(old_drm_plane_state); >> + new_plane_state = to_rcar_plane_state(new_drm_plane_state); >> >> dev_dbg(rcdu->dev, "%s: allocating plane (%u,%tu)\n", __func__, >> plane->group->index, plane - plane->group->planes); >> @@ -263,7 +262,7 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, >> * reallocated. >> */ >> if (!plane_state->format || > I think you missed a few cases of replacing plane_state here. Does this > compile? I fear not.. Have a working patch below. :) ----8<------ for_each_obj_in_state is about to be removed, so use the correct new iterator macros. Also look at new_plane_state instead of plane->state when looking up the hw planes in use. They should be the same except when reallocating, (in which case this code is skipped) and we should really stop looking at obj->state whenever possible. Changes since v1: - Actually compile correctly. Signed-off-by: Maarten Lankhorst Cc: Laurent Pinchart Cc: linux-renesas-soc@vger.kernel.org --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 57 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index dcde6288da6c..50fd793c38d1 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -51,12 +51,9 @@ */ static bool rcar_du_plane_needs_realloc(struct rcar_du_plane *plane, + const struct rcar_du_plane_state *cur_state, struct rcar_du_plane_state *new_state) { - struct rcar_du_plane_state *cur_state; - - cur_state = to_rcar_plane_state(plane->plane.state); - /* Lowering the number of planes doesn't strictly require reallocation * as the extra hardware plane will be freed when committing, but doing * so could lead to more fragmentation. @@ -141,16 +138,17 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, unsigned int groups = 0; unsigned int i; struct drm_plane *drm_plane; - struct drm_plane_state *drm_plane_state; + struct drm_plane_state *old_drm_plane_state, *new_drm_plane_state; /* Check if hardware planes need to be reallocated. */ - for_each_plane_in_state(state, drm_plane, drm_plane_state, i) { - struct rcar_du_plane_state *plane_state; + for_each_oldnew_plane_in_state(state, drm_plane, old_drm_plane_state, new_drm_plane_state, i) { + struct rcar_du_plane_state *old_plane_state, *new_plane_state; struct rcar_du_plane *plane; unsigned int index; plane = to_rcar_plane(drm_plane); - plane_state = to_rcar_plane_state(drm_plane_state); + old_plane_state = to_rcar_plane_state(old_drm_plane_state); + new_plane_state = to_rcar_plane_state(new_drm_plane_state); dev_dbg(rcdu->dev, "%s: checking plane (%u,%tu)\n", __func__, plane->group->index, plane - plane->group->planes); @@ -159,19 +157,19 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, * the full reallocation procedure. Just mark the hardware * plane(s) as freed. */ - if (!plane_state->format) { + if (!new_plane_state->format) { dev_dbg(rcdu->dev, "%s: plane is being disabled\n", __func__); index = plane - plane->group->planes; group_freed_planes[plane->group->index] |= 1 << index; - plane_state->hwindex = -1; + new_plane_state->hwindex = -1; continue; } /* If the plane needs to be reallocated mark it as such, and * mark the hardware plane(s) as free. */ - if (rcar_du_plane_needs_realloc(plane, plane_state)) { + if (rcar_du_plane_needs_realloc(plane, old_plane_state, new_plane_state)) { dev_dbg(rcdu->dev, "%s: plane needs reallocation\n", __func__); groups |= 1 << plane->group->index; @@ -179,7 +177,7 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, index = plane - plane->group->planes; group_freed_planes[plane->group->index] |= 1 << index; - plane_state->hwindex = -1; + new_plane_state->hwindex = -1; } } @@ -204,7 +202,7 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, for (i = 0; i < group->num_planes; ++i) { struct rcar_du_plane *plane = &group->planes[i]; - struct rcar_du_plane_state *plane_state; + struct rcar_du_plane_state *new_plane_state; struct drm_plane_state *s; s = drm_atomic_get_plane_state(state, &plane->plane); @@ -226,16 +224,16 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, continue; } - plane_state = to_rcar_plane_state(plane->plane.state); - used_planes |= rcar_du_plane_hwmask(plane_state); + new_plane_state = to_rcar_plane_state(s); + used_planes |= rcar_du_plane_hwmask(new_plane_state); dev_dbg(rcdu->dev, "%s: plane (%u,%tu) uses %u hwplanes (index %d)\n", __func__, plane->group->index, plane - plane->group->planes, - plane_state->format ? - plane_state->format->planes : 0, - plane_state->hwindex); + new_plane_state->format ? + new_plane_state->format->planes : 0, + new_plane_state->hwindex); } group_free_planes[index] = 0xff & ~used_planes; @@ -246,15 +244,16 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, } /* Reallocate hardware planes for each plane that needs it. */ - for_each_plane_in_state(state, drm_plane, drm_plane_state, i) { - struct rcar_du_plane_state *plane_state; + for_each_oldnew_plane_in_state(state, drm_plane, old_drm_plane_state, new_drm_plane_state, i) { + struct rcar_du_plane_state *old_plane_state, *new_plane_state; struct rcar_du_plane *plane; unsigned int crtc_planes; unsigned int free; int idx; plane = to_rcar_plane(drm_plane); - plane_state = to_rcar_plane_state(drm_plane_state); + old_plane_state = to_rcar_plane_state(old_drm_plane_state); + new_plane_state = to_rcar_plane_state(new_drm_plane_state); dev_dbg(rcdu->dev, "%s: allocating plane (%u,%tu)\n", __func__, plane->group->index, plane - plane->group->planes); @@ -262,8 +261,8 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, /* Skip planes that are being disabled or don't need to be * reallocated. */ - if (!plane_state->format || - !rcar_du_plane_needs_realloc(plane, plane_state)) + if (!new_plane_state->format || + !rcar_du_plane_needs_realloc(plane, old_plane_state, new_plane_state)) continue; /* Try to allocate the plane from the free planes currently @@ -271,15 +270,15 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, * group and thus minimize flicker. If it fails fall back to * allocating from all free planes. */ - crtc_planes = to_rcar_crtc(plane_state->state.crtc)->index % 2 + crtc_planes = to_rcar_crtc(new_plane_state->state.crtc)->index % 2 ? plane->group->dptsr_planes : ~plane->group->dptsr_planes; free = group_free_planes[plane->group->index]; - idx = rcar_du_plane_hwalloc(plane, plane_state, + idx = rcar_du_plane_hwalloc(plane, new_plane_state, free & crtc_planes); if (idx < 0) - idx = rcar_du_plane_hwalloc(plane, plane_state, + idx = rcar_du_plane_hwalloc(plane, new_plane_state, free); if (idx < 0) { dev_dbg(rcdu->dev, "%s: no available hardware plane\n", @@ -288,12 +287,12 @@ int rcar_du_atomic_check_planes(struct drm_device *dev, } dev_dbg(rcdu->dev, "%s: allocated %u hwplanes (index %u)\n", - __func__, plane_state->format->planes, idx); + __func__, new_plane_state->format->planes, idx); - plane_state->hwindex = idx; + new_plane_state->hwindex = idx; group_free_planes[plane->group->index] &= - ~rcar_du_plane_hwmask(plane_state); + ~rcar_du_plane_hwmask(new_plane_state); dev_dbg(rcdu->dev, "%s: group %u free planes mask 0x%02x\n", __func__, plane->group->index,