Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <b3855e51-6e44-402e-9727-d67ac6ae64b5@samsung.com>
Date:   Mon, 4 Dec 2023 17:05:33 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 3/3] drm/panfrost: Synchronize and disable interrupts
 before powering off
Content-Language: en-US
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        boris.brezillon@collabora.com
Cc:     robh@kernel.org, steven.price@arm.com,
        maarten.lankhorst@linux.intel.com, mripard@kernel.org,
        tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        kernel@collabora.com, krzysztof.kozlowski@linaro.org
From:   Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <20231204114215.54575-4-angelogioacchino.delregno@collabora.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20231204114215.54575-1-angelogioacchino.delregno@collabora.com>
        <CGME20231204114229eucas1p22c7ef58046923b0de57a0cc139a94afc@eucas1p2.samsung.com>
        <20231204114215.54575-4-angelogioacchino.delregno@collabora.com>
Precedence: bulk

On 04.12.2023 12:42, AngeloGioacchino Del Regno wrote:
> To make sure that we don't unintentionally perform any unclocked and/or
> unpowered R/W operation on GPU registers, before turning off clocks and
> regulators we must make sure that no GPU, JOB or MMU ISR execution is
> pending: doing that requires to add a mechanism to synchronize the
> interrupts on suspend.
>
> Add functions panfrost_{gpu,job,mmu}_suspend_irq() which will perform
> interrupts masking and ISR execution synchronization, and then call
> those in the panfrost_device_runtime_suspend() handler in the exact
> sequence of job (may require mmu!) -> mmu -> gpu.
>
> As a side note, JOB and MMU suspend_irq functions needed some special
> treatment: as their interrupt handlers will unmask interrupts, it was
> necessary to add an `is_suspended` bitmap which is used to address the
> possible corner case of unintentional IRQ unmasking because of ISR
> execution after a call to synchronize_irq().
>
> At resume, clear each is_suspended bit in the reset path of JOB/MMU
> to allow unmasking the interrupts.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>


> ---
>   drivers/gpu/drm/panfrost/panfrost_device.c |  3 +++
>   drivers/gpu/drm/panfrost/panfrost_device.h |  8 +++++++
>   drivers/gpu/drm/panfrost/panfrost_gpu.c    | 18 +++++++++++++--
>   drivers/gpu/drm/panfrost/panfrost_gpu.h    |  1 +
>   drivers/gpu/drm/panfrost/panfrost_job.c    | 26 ++++++++++++++++++----
>   drivers/gpu/drm/panfrost/panfrost_job.h    |  1 +
>   drivers/gpu/drm/panfrost/panfrost_mmu.c    | 22 +++++++++++++++---
>   drivers/gpu/drm/panfrost/panfrost_mmu.h    |  1 +
>   8 files changed, 71 insertions(+), 9 deletions(-)
>
> ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland