From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Paul Subject: Re: [PATCH v3 02/32] drm/exynos: Merge overlay_ops into manager_ops Date: Fri, 1 Nov 2013 15:50:05 -0400 Message-ID: References: <1383063198-10526-1-git-send-email-seanpaul@chromium.org> <1383063198-10526-3-git-send-email-seanpaul@chromium.org> <1403888.Y8mcJtf5WK@flatron> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-yh0-f42.google.com (mail-yh0-f42.google.com [209.85.213.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 2A57AF01BB for ; Fri, 1 Nov 2013 12:50:29 -0700 (PDT) Received: by mail-yh0-f42.google.com with SMTP id z6so2103868yhz.15 for ; Fri, 01 Nov 2013 12:50:28 -0700 (PDT) Received: from mail-ie0-x236.google.com (mail-ie0-x236.google.com [2607:f8b0:4001:c03::236]) by mx.google.com with ESMTPSA id g25sm6847634yhg.6.2013.11.01.12.50.26 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 01 Nov 2013 12:50:26 -0700 (PDT) Received: by mail-ie0-f182.google.com with SMTP id as1so8224780iec.41 for ; Fri, 01 Nov 2013 12:50:26 -0700 (PDT) In-Reply-To: <1403888.Y8mcJtf5WK@flatron> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Tomasz Figa Cc: dri-devel , =?ISO-8859-1?Q?St=E9phane_Marchesin?= List-Id: dri-devel@lists.freedesktop.org On Thu, Oct 31, 2013 at 7:39 PM, Tomasz Figa wrote: > Hi Sean, > > On Tuesday 29 of October 2013 12:12:48 Sean Paul wrote: >> This patch merges overlay_ops into manager_ops. In all cases, >> overlay_ops is implemented in the same place as manager ops, it doesn't >> serve a functional purpose, and doesn't make things more clear. >> >> Signed-off-by: Sean Paul >> --- >> >> Changes in v2: None >> Changes in v3: None >> >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 29 +-- >> drivers/gpu/drm/exynos/exynos_drm_encoder.c | 26 +- >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 363 >> ++++++++++++++-------------- drivers/gpu/drm/exynos/exynos_drm_hdmi.c >> | 36 ++- >> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 29 +-- >> drivers/gpu/drm/exynos/exynos_mixer.c | 2 - >> 6 files changed, 229 insertions(+), 256 deletions(-) > [snip] >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 868a14d..f271f22 >> 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -181,185 +181,6 @@ static struct exynos_drm_display_ops >> fimd_display_ops = { .power_on = fimd_display_power_on, >> }; >> >> -static void fimd_dpms(struct device *subdrv_dev, int mode) >> -{ >> - struct fimd_context *ctx = get_fimd_context(subdrv_dev); >> - >> - DRM_DEBUG_KMS("%d\n", mode); >> - >> - mutex_lock(&ctx->lock); >> - >> - switch (mode) { >> - case DRM_MODE_DPMS_ON: >> - /* >> - * enable fimd hardware only if suspended status. >> - * >> - * P.S. fimd_dpms function would be called at booting time > so >> - * clk_enable could be called double time. >> - */ >> - if (ctx->suspended) >> - pm_runtime_get_sync(subdrv_dev); >> - break; >> - case DRM_MODE_DPMS_STANDBY: >> - case DRM_MODE_DPMS_SUSPEND: >> - case DRM_MODE_DPMS_OFF: >> - if (!ctx->suspended) >> - pm_runtime_put_sync(subdrv_dev); >> - break; >> - default: >> - DRM_DEBUG_KMS("unspecified mode %d\n", mode); >> - break; >> - } >> - >> - mutex_unlock(&ctx->lock); >> -} > [snip] >> -static void fimd_wait_for_vblank(struct device *dev) >> -{ >> - struct fimd_context *ctx = get_fimd_context(dev); >> - >> - if (ctx->suspended) >> - return; >> - >> - atomic_set(&ctx->wait_vsync_event, 1); >> - >> - /* >> - * wait for FIMD to signal VSYNC interrupt or return after >> - * timeout which is set to 50ms (refresh rate of 20). >> - */ >> - if (!wait_event_timeout(ctx->wait_vsync_queue, >> - !atomic_read(&ctx->wait_vsync_event), >> - DRM_HZ/20)) >> - DRM_DEBUG_KMS("vblank wait timed out.\n"); >> -} > > Do you need all the churn of moving all the functions above? I believe it > would be enough to simply move the structure. This would greatly decrease > the diffstat and chances of possible merge conflicts. > Hi Tomasz, I've done as you suggest, I'll post a new version once we settle on the other issues you brought up. Sean > Otherwise the patch looks good to me and improves things indeed. > > Best regards, > Tomasz >