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;
Received-SPF: Pass (protection.outlook.com: domain of arm.com designates
 63.35.35.123 as permitted sender) receiver=protection.outlook.com;
 client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com;
From:   Mihail Atanassov <Mihail.Atanassov@arm.com>
To:     james qian wang <james.qian.wang@arm.com>
CC:     nd <nd@arm.com>, Jonathan Chai <Jonathan.Chai@arm.com>,
        Lowry Li <Lowry.Li@arm.com>, Tiannan Zhu <Tiannan.Zhu@arm.com>,
        nd <nd@arm.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        Ben Davis <Ben.Davis@arm.com>,
        Oscar Zhang <Oscar.Zhang@arm.com>,
        Channing Chen <Channing.Chen@arm.com>,
        james qian wang <james.qian.wang@arm.com>
Subject: Re: [PATCH] drm/komeda: Add runtime_pm support
Thread-Topic: [PATCH] drm/komeda: Add runtime_pm support
Thread-Index: AQHVsMCBN2D7zCEwDUuoMKQ83lPGMqfHnBGA
Date:   Mon, 23 Dec 2019 13:02:13 +0000
Message-ID: <1985918.FRP2JxviHk@e123338-lin>
References: <20191212074756.14678-1-james.qian.wang@arm.com>
In-Reply-To: <20191212074756.14678-1-james.qian.wang@arm.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
Authentication-Results-Original: spf=none (sender IP is )
 smtp.mailfrom=Mihail.Atanassov@arm.com; 
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-publictraffictype: Email
X-MS-Office365-Filtering-HT: Tenant
X-MS-Office365-Filtering-Correlation-Id: d564c9b7-8412-4c76-e4b0-08d787a859f0
X-MS-TrafficTypeDiagnostic: VI1PR08MB3760:|VI1PR08MB3760:|VI1PR08MB3790:
x-ms-exchange-transport-forked: True
X-Microsoft-Antispam-PRVS: <VI1PR08MB379051CC449786B9B28881E08F2E0@VI1PR08MB3790.eurprd08.prod.outlook.com>
x-checkrecipientrouted: true
x-ms-oob-tlc-oobclassifiers: OLM:4125;OLM:4125;
x-forefront-prvs: 0260457E99
X-Forefront-Antispam-Report-Untrusted: SFV:NSPM;SFS:(10009020)(7916004)(4636009)(396003)(376002)(39860400002)(346002)(366004)(136003)(189003)(199004)(478600001)(81166006)(8676002)(2906002)(6512007)(9686003)(8936002)(316002)(4326008)(54906003)(6506007)(81156014)(6862004)(33716001)(6486002)(6636002)(86362001)(52116002)(186003)(5660300002)(66946007)(64756008)(66476007)(66446008)(71200400001)(26005)(66556008)(39026012);DIR:OUT;SFP:1101;SCL:1;SRVR:VI1PR08MB3760;H:VI1PR08MB4078.eurprd08.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1;
received-spf: None (protection.outlook.com: arm.com does not designate
 permitted sender hosts)
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam-Untrusted: BCL:0;
X-Microsoft-Antispam-Message-Info-Original: of0FJK0Da8PhkaNI6YCE+1e5/tVw6FKlaIMvYKXu0gLh8UPOO98+D3ygtLUGnh4QlbON6lgXbLm52CznLr6netMQtneDBLQHGLfX5rb+SE+4xcoRpNLqG4kr679uxV1dXikKGQI2JJi/Tri1Q2gOLStkGVKC/XeE01JD6q4u8YisRJL9FSJ5HxLMzwoEhj7NNZkNVCs4VVykazApA77djSorprUCelwO5UxZAijujHk7iFdODFasGswQpGBz3dHggldRXaQezL5VyFwdDqquwcJxPL7/BBYfauz1xJs6TEmqEsiOAbMzw3qOK1RNLbKv9V8PriWCt68aLcMCfqVD6+DxMvtsbYQAojMfG6AslnEyHehB0nuSCCDz2A5WuVzf8X28vR9MRhZIPk8BXIN+rVx89EqILipHE8CJppcS7tVasFcKaF2cXKEDAkxQyYJOSZQFbZ3uugYRFmtt8hh826zmKomS9oFjCQZqvp0nk3CNJ5HtyQtyBDdfR6kYX8wW
Content-Type: text/plain; charset="us-ascii"
Content-ID: <EBFC22F6A14A3A4E90036BBA0C4D2FF7@eurprd08.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB3760
Original-Authentication-Results: spf=none (sender IP is )
 smtp.mailfrom=Mihail.Atanassov@arm.com; 
X-EOPAttributedMessage: 0
X-MS-Exchange-Transport-CrossTenantHeadersStripped: DB5EUR03FT053.eop-EUR03.prod.protection.outlook.com
X-Forefront-Antispam-Report: CIP:63.35.35.123;IPV:CAL;SCL:-1;CTRY:IE;EFV:NLI;SFV:NSPM;SFS:(10009020)(4636009)(7916004)(136003)(396003)(376002)(346002)(39860400002)(199004)(189003)(26005)(9686003)(6512007)(8676002)(86362001)(81166006)(81156014)(6636002)(2906002)(186003)(33716001)(6862004)(8936002)(54906003)(70206006)(70586007)(316002)(76130400001)(26826003)(6506007)(356004)(4326008)(5660300002)(478600001)(336012)(6486002)(39026012);DIR:OUT;SFP:1101;SCL:1;SRVR:VI1PR08MB3790;H:64aa7808-outbound-1.mta.getcheckrecipient.com;FPR:;SPF:Pass;LANG:en;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;A:1;MX:1;
X-MS-Office365-Filtering-Correlation-Id-Prvs: f3e3eb53-ece9-4b1e-88ae-08d787a85470
NoDisclaimer: True
X-Forefront-PRVS: 0260457E99
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: O1I8rRCigTEZLFfP8XcZmlJU795Q5SGjuFQt6Ar3R4CI01RDFqRBihZn4eCyeNsoFCrxrcE2RbINdod9dxRVzam8hP8a/eeWmT3grRxtPfk0ay7rAKbn8OglzcceDOgdXCYCDPvH7ZTkEjJUneec8LSdFNTrF9/qWGBynwGLHBcVL21krGaZBXUaas1wlb1SB7yhuNPnlQrzPh20NijVsOughWQVs1acMNSiJ9rOhiVbDB2vMSMCN1Bke6llPckQ4NDQofscoCSC38W6OG5PmAOjBnPXol3N0wvlYfQ5xvzz+MARWSpgHFFO434bnIx7lWOLst2UKyfiQqmwEA1NdkiwX47lkcV7T/DTbDFUbMC6k2luLQQWtayl+KwDhowU87V9z+mpYL2uTArPs+uRQXJqcDTFubscNtvQJKV4FIB9YtSZnCZhTNkhRgv/9475IlM6LBzW2+UOLfKZ5Bajr1GrjmSs9BjkqhO9JUkBw6QQdR14mnKA+q5c7Xptnt5M
X-OriginatorOrg: arm.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Dec 2019 13:02:22.8678
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: d564c9b7-8412-4c76-e4b0-08d787a859f0
X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB3790
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thursday, 12 December 2019 07:48:13 GMT james qian wang (Arm Technology =
China) wrote:
> - Add pm_runtime_get/put to crtc_enable/disable along with the real
>   display usage
> - Add runtime_get/put to register_show, since register_show() will
>   access register, need to wakeup HW.
> - For the case that PM is not enabled or configured, manually wakeup HW
>=20
> Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@ar=
m.com>
> ---
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  3 +
>  .../gpu/drm/arm/display/komeda/komeda_dev.c   | 55 +++++--------------
>  .../gpu/drm/arm/display/komeda/komeda_drv.c   | 42 ++++++++++++--
>  .../gpu/drm/arm/display/komeda/komeda_kms.c   |  6 --
>  4 files changed, 53 insertions(+), 53 deletions(-)
>=20
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/g=
pu/drm/arm/display/komeda/komeda_crtc.c
> index 1c452ea75999..56bd938961ee 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -5,6 +5,7 @@
>   *
>   */
>  #include <linux/clk.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/spinlock.h>
> =20
>  #include <drm/drm_atomic.h>
> @@ -274,6 +275,7 @@ static void
>  komeda_crtc_atomic_enable(struct drm_crtc *crtc,
>  			  struct drm_crtc_state *old)
>  {
> +	pm_runtime_get_sync(crtc->dev->dev);
>  	komeda_crtc_prepare(to_kcrtc(crtc));
>  	drm_crtc_vblank_on(crtc);
>  	WARN_ON(drm_crtc_vblank_get(crtc));
> @@ -372,6 +374,7 @@ komeda_crtc_atomic_disable(struct drm_crtc *crtc,
>  	drm_crtc_vblank_put(crtc);
>  	drm_crtc_vblank_off(crtc);
>  	komeda_crtc_unprepare(kcrtc);
> +	pm_runtime_put(crtc->dev->dev);
>  }
> =20
>  static void
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gp=
u/drm/arm/display/komeda/komeda_dev.c
> index 38b832804bad..1d767473ba8a 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -10,6 +10,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/dma-mapping.h>
>  #ifdef CONFIG_DEBUG_FS
>  #include <linux/debugfs.h>
> @@ -27,12 +28,16 @@ static int komeda_register_show(struct seq_file *sf, =
void *x)
> =20
>  	seq_puts(sf, "\n=3D=3D=3D=3D=3D=3D Komeda register dump =3D=3D=3D=3D=3D=
=3D=3D=3D=3D\n");
> =20
> +	pm_runtime_get_sync(mdev->dev);
> +
>  	if (mdev->funcs->dump_register)
>  		mdev->funcs->dump_register(mdev, sf);
> =20
>  	for (i =3D 0; i < mdev->n_pipelines; i++)
>  		komeda_pipeline_dump_register(mdev->pipelines[i], sf);
> =20
> +	pm_runtime_put(mdev->dev);
> +
>  	return 0;
>  }
> =20
> @@ -263,15 +268,6 @@ struct komeda_dev *komeda_dev_create(struct device *=
dev)
>  	if (!mdev->iommu)
>  		DRM_INFO("continue without IOMMU support!\n");
> =20
> -	if (mdev->iommu && mdev->funcs->connect_iommu) {
> -		err =3D mdev->funcs->connect_iommu(mdev);
> -		if (err) {
> -			DRM_ERROR("connect iommu failed.\n");
> -			mdev->iommu =3D NULL;
> -			goto disable_clk;
> -		}
> -	}
> -
>  	clk_disable_unprepare(mdev->aclk);
> =20
>  	err =3D sysfs_create_group(&dev->kobj, &komeda_sysfs_attr_group);
> @@ -310,11 +306,6 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  	if (mdev->aclk)
>  		clk_prepare_enable(mdev->aclk);
> =20
> -	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> -		if (mdev->funcs->disconnect_iommu(mdev))
> -			DRM_ERROR("disconnect iommu failed.\n");
> -	mdev->iommu =3D NULL;
> -
>  	for (i =3D 0; i < mdev->n_pipelines; i++) {
>  		komeda_pipeline_destroy(mdev, mdev->pipelines[i]);
>  		mdev->pipelines[i] =3D NULL;
> @@ -343,44 +334,26 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
> =20
>  int komeda_dev_resume(struct komeda_dev *mdev)
>  {
> -	int ret =3D 0;
> -
>  	clk_prepare_enable(mdev->aclk);
> =20
> -	if (mdev->iommu && mdev->funcs->connect_iommu) {
> -		ret =3D mdev->funcs->connect_iommu(mdev);
> -		if (ret < 0) {
> -			DRM_ERROR("connect iommu failed.\n");
> -			goto disable_clk;
> -		}
> -	}
> -
> -	ret =3D mdev->funcs->enable_irq(mdev);
> +	mdev->funcs->enable_irq(mdev);
> =20
> -disable_clk:
> -	clk_disable_unprepare(mdev->aclk);
> +	if (mdev->iommu && mdev->funcs->connect_iommu)
> +		if (mdev->funcs->connect_iommu(mdev))
> +			DRM_ERROR("connect iommu failed.\n");
> =20
> -	return ret;
> +	return 0;
>  }
> =20
>  int komeda_dev_suspend(struct komeda_dev *mdev)
>  {
> -	int ret =3D 0;
> -
> -	clk_prepare_enable(mdev->aclk);
> -
> -	if (mdev->iommu && mdev->funcs->disconnect_iommu) {
> -		ret =3D mdev->funcs->disconnect_iommu(mdev);
> -		if (ret < 0) {
> +	if (mdev->iommu && mdev->funcs->disconnect_iommu)
> +		if (mdev->funcs->disconnect_iommu(mdev))
>  			DRM_ERROR("disconnect iommu failed.\n");
> -			goto disable_clk;
> -		}
> -	}
> =20
> -	ret =3D mdev->funcs->disable_irq(mdev);
> +	mdev->funcs->disable_irq(mdev);
> =20
> -disable_clk:
>  	clk_disable_unprepare(mdev->aclk);
> =20
> -	return ret;
> +	return 0;
>  }
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gp=
u/drm/arm/display/komeda/komeda_drv.c
> index ad38bbc7431e..ea5cd1e17304 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> @@ -33,6 +33,12 @@ static void komeda_unbind(struct device *dev)
>  		return;
> =20
>  	komeda_kms_detach(mdrv->kms);
> +
> +	if (pm_runtime_enabled(dev))
> +		pm_runtime_disable(dev);
> +	else
> +		komeda_dev_suspend(mdrv->mdev);
> +
>  	komeda_dev_destroy(mdrv->mdev);
> =20
>  	dev_set_drvdata(dev, NULL);
> @@ -54,6 +60,10 @@ static int komeda_bind(struct device *dev)
>  		goto free_mdrv;
>  	}
> =20
> +	pm_runtime_enable(dev);
> +	if (!pm_runtime_enabled(dev))
> +		komeda_dev_resume(mdrv->mdev);
> +
>  	mdrv->kms =3D komeda_kms_attach(mdrv->mdev);
>  	if (IS_ERR(mdrv->kms)) {
>  		err =3D PTR_ERR(mdrv->kms);
> @@ -65,6 +75,11 @@ static int komeda_bind(struct device *dev)
>  	return 0;
> =20
>  destroy_mdev:
> +	if (pm_runtime_enabled(dev))
> +		pm_runtime_disable(dev);
> +	else
> +		komeda_dev_suspend(mdrv->mdev);
> +
>  	komeda_dev_destroy(mdrv->mdev);
> =20
>  free_mdrv:
> @@ -131,15 +146,29 @@ static const struct of_device_id komeda_of_match[] =
=3D {
> =20
>  MODULE_DEVICE_TABLE(of, komeda_of_match);
> =20
> +static int komeda_rt_pm_suspend(struct device *dev)
> +{
> +	struct komeda_drv *mdrv =3D dev_get_drvdata(dev);
> +
> +	return komeda_dev_suspend(mdrv->mdev);
> +}
> +
> +static int komeda_rt_pm_resume(struct device *dev)
> +{
> +	struct komeda_drv *mdrv =3D dev_get_drvdata(dev);
> +
> +	return komeda_dev_resume(mdrv->mdev);
> +}
> +
>  static int __maybe_unused komeda_pm_suspend(struct device *dev)
>  {
>  	struct komeda_drv *mdrv =3D dev_get_drvdata(dev);
> -	struct drm_device *drm =3D &mdrv->kms->base;
>  	int res;
> =20
> -	res =3D drm_mode_config_helper_suspend(drm);
> +	res =3D drm_mode_config_helper_suspend(&mdrv->kms->base);
> =20
> -	komeda_dev_suspend(mdrv->mdev);
> +	if (!pm_runtime_status_suspended(dev))
> +		komeda_dev_suspend(mdrv->mdev);
> =20
>  	return res;
>  }
> @@ -147,15 +176,16 @@ static int __maybe_unused komeda_pm_suspend(struct =
device *dev)
>  static int __maybe_unused komeda_pm_resume(struct device *dev)
>  {
>  	struct komeda_drv *mdrv =3D dev_get_drvdata(dev);
> -	struct drm_device *drm =3D &mdrv->kms->base;
> =20
> -	komeda_dev_resume(mdrv->mdev);
> +	if (!pm_runtime_status_suspended(dev))
> +		komeda_dev_resume(mdrv->mdev);
> =20
> -	return drm_mode_config_helper_resume(drm);
> +	return drm_mode_config_helper_resume(&mdrv->kms->base);
>  }
> =20
>  static const struct dev_pm_ops komeda_pm_ops =3D {
>  	SET_SYSTEM_SLEEP_PM_OPS(komeda_pm_suspend, komeda_pm_resume)
> +	SET_RUNTIME_PM_OPS(komeda_rt_pm_suspend, komeda_rt_pm_resume, NULL)
>  };
> =20
>  static struct platform_driver komeda_platform_driver =3D {
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gp=
u/drm/arm/display/komeda/komeda_kms.c
> index e30a5b43caa9..9a7dcf92591a 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -307,10 +307,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct kome=
da_dev *mdev)
>  	if (err)
>  		goto free_component_binding;
> =20
> -	err =3D mdev->funcs->enable_irq(mdev);
> -	if (err)
> -		goto free_component_binding;
> -
>  	drm->irq_enabled =3D true;
> =20
>  	drm_kms_helper_poll_init(drm);
> @@ -324,7 +320,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komed=
a_dev *mdev)
>  free_interrupts:
>  	drm_kms_helper_poll_fini(drm);
>  	drm->irq_enabled =3D false;
> -	mdev->funcs->disable_irq(mdev);
>  free_component_binding:
>  	component_unbind_all(mdev->dev, drm);
>  cleanup_mode_config:
> @@ -346,7 +341,6 @@ void komeda_kms_detach(struct komeda_kms_dev *kms)
>  	drm_kms_helper_poll_fini(drm);
>  	drm_atomic_helper_shutdown(drm);
>  	drm->irq_enabled =3D false;
> -	mdev->funcs->disable_irq(mdev);
>  	component_unbind_all(mdev->dev, drm);
>  	drm_mode_config_cleanup(drm);
>  	komeda_kms_cleanup_private_objs(kms);
>=20

Reviewed-by: Mihail Atanassov <mihail.atanassov@arm.com>

--=20
Mihail