Received: by 2002:ac0:a5a7:0:0:0:0:0 with SMTP id m36-v6csp2282133imm; Sat, 28 Jul 2018 13:20:39 -0700 (PDT) X-Google-Smtp-Source: AAOMgpc1XBxxyWx7Q/BKc6zz+bR9CHbmn4vJkfiz37j5Xfr5+k/KbOu2WQ1F/Agg8Ji/Wev81jLs X-Received: by 2002:aa7:8591:: with SMTP id w17-v6mr11879971pfn.77.1532809239623; Sat, 28 Jul 2018 13:20:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1532809239; cv=none; d=google.com; s=arc-20160816; b=NRJnpNMDzp5SA4O3YD+UwyjbJdKsTaLAQFCWKA8mg//1Lh+os1sh+iozKB3K5au1z0 9OML0POwRyZtAKQNltZcxOs5aOM5GlghGbjjTFGretfQ1E9CRU0QilyFRMD95tKeCjK7 TC5WgK+eiRLXExrVg2CVdXeYwADA7mCW1iCCia2ZjAvSTFCRuHJ+QfyVMcsM22yHndg8 12NpBc700l2y2P63LVJGM+BluYScQqALAiy2/aSuEQqFwM6p5Y5X7bMCeNUcY2BVPUmw TO3VreGuH3+6MWatq/r5vLs5OpX6JVbzeeSkyePVKv4tCVWhbMSfn3qkwO8MSlEYrgkg JNrA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:organization:message-id:date:subject:cc:to :from:dkim-signature:arc-authentication-results; bh=p+Jdbx4mZQw0MrdQGDLK+rCeLE5//C1lzYVqisfDIU8=; b=gz87ptvl+BdpZdobA17BKjqe0SDTRLl5d+akdVM+NE9FkT481aMyQK/yqhxcDi3WPy 5Rx9+b8smlt0bWejfNgjP0dYpaL8U4gapjGrV7K4nrSFn5NwSqpnr1P7FvwL9HubFqwk iLxz8IbpDt5NmuJQ8R8RAfjj+M5F4/orebUcyTSgaIkcQc+QuRz5XpDtdM8RuJ9UPgyi CvNEEW4HFvjYQ4k3IV5DXQYrzqPcS5EyusBcFnl4SrSRHqGC7sBRchPFW7R6VtcBqdJB 3V+p1XLCc13ODtR8PeZn3+Oez0w9B6UpGKojqstZYkQuh5GpIPUad9bkXnS94gFv0X92 26NA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=Fdqbv8pi; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j6-v6si1092209plk.438.2018.07.28.13.20.23; Sat, 28 Jul 2018 13:20:39 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=Fdqbv8pi; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730785AbeG1VrO (ORCPT + 99 others); Sat, 28 Jul 2018 17:47:14 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:52742 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730518AbeG1VrO (ORCPT ); Sat, 28 Jul 2018 17:47:14 -0400 Received: from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8AD1391; Sat, 28 Jul 2018 22:19:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1532809172; bh=khjjzhamd+6LbLJdrLKmp+swQxpZ8dF3NPp67nE1qkU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Fdqbv8pigQaSIOrxpquW/tdP+w7uoFgjpQwxA86TUIvFSezunDnlxNbj9rigPSfC/ ieqEQwesIFDA7O0wkwYPJd6F9rb5qCHfU2L1SgXbz4UBgerzzvRyVu8/4wHpC/nNvp K/HKwhl2tv0joh1lDs8E4M2bdTEkVGtKh2bg9b/E= From: Laurent Pinchart To: Souptick Joarder Cc: Vaishali Thakkar , airlied@linux.ie, Ajit Linux , dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, Linux Kernel Mailing List , Daniel Vetter Subject: Re: [PATCH] drm/rcar-du: Convert drm_atomic_helper_suspend/resume() Date: Sat, 28 Jul 2018 23:20:07 +0300 Message-ID: <10502231.oSnn9dME1M@avalon> Organization: Ideas on Board Oy In-Reply-To: References: <20180728154007.GA28426@jordon-HP-15-Notebook-PC> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Souptick, Thank you for the patch. On Saturday, 28 July 2018 21:50:58 EEST Souptick Joarder wrote: > On Sat, Jul 28, 2018 at 11:20 PM, Vaishali Thakkar wrote: > > On Sat, Jul 28, 2018 at 9:10 PM, Souptick Joarder wrote: > >> convert drm_atomic_helper_suspend/resume() to use > >> drm_mode_config_helper_suspend/resume(). > > > > Hi Souptick, > > > > Thanks for your patch. > > > >> Signed-off-by: Souptick Joarder > >> Signed-off-by: Ajit Negi > >> --- > >> > >> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 ++------------------- > >> 1 file changed, 2 insertions(+), 19 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 02aee6c..288220f 100644 > >> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> @@ -357,32 +357,15 @@ static void rcar_du_lastclose(struct drm_device > >> *dev) > >> > >> static int rcar_du_pm_suspend(struct device *dev) > >> { > >> struct rcar_du_device *rcdu = dev_get_drvdata(dev); > >> - struct drm_atomic_state *state; > >> > >> - drm_kms_helper_poll_disable(rcdu->ddev); > >> - drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, true); > >> - > >> - state = drm_atomic_helper_suspend(rcdu->ddev); > >> - if (IS_ERR(state)) { > >> - drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false); > > > > I don't think we can use drm_mode_config_helper_(suspend/resume) > > API here as this file uses CMA functions. > > drm_fbdev_cma_set_suspend_unlocked() is wrapper function which > invokes drm_fb_helper_set_suspend_unlocked(). > > Where the new API drm_mode_config_helper_suspend/resume() directly invokes > drm_fb_helper_set_suspend_unlocked(). So it is safe to replace exiting > code with API drm_mode_config_helper_suspend/resume(). I agree that they're functionally equivalent for now, but what if drm_fbdev_cma_set_suspend_unlocked() gets extended later ? This change risks introducing a breakage that could could unnoticed at that point. At the very least you should add a comment in drm_fbdev_cma_set_suspend_unlocked() to explain that any extension of the function should also address all drivers using drm_mode_config_helper_suspend() and drm_mode_config_helper_resume(). > > And from git grep it seems that there are very few drivers using it at the > > moment, so not sure if introducing new API functions similar to > > drm_mode_config will make sense or not. > > https://www.kernel.org/doc/html/latest/gpu/todo.html > > It was picked up from TODO list after discussing with > Daniel. > > > Thanks. > > > >> - drm_kms_helper_poll_enable(rcdu->ddev); > >> - return PTR_ERR(state); > >> - } > >> - > >> - rcdu->suspend_state = state; Additionally, I think you can remove the suspend_state field from the rcdu structure. > >> - return 0; > >> + return drm_mode_config_helper_suspend(rcdu->ddev); > >> } > >> > >> static int rcar_du_pm_resume(struct device *dev) > >> { > >> struct rcar_du_device *rcdu = dev_get_drvdata(dev); > >> > >> - drm_atomic_helper_resume(rcdu->ddev, rcdu->suspend_state); > >> - drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false); > >> - drm_kms_helper_poll_enable(rcdu->ddev); > >> - > >> - return 0; > >> + return drm_mode_config_helper_resume(rcdu->ddev); > >> } > >> #endif -- Regards, Laurent Pinchart