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;
Subject: Re: [PATCH v6 28/30] drm/rockchip: Disable PSR from reboot notifier
To:     Enric Balletbo i Serra <enric.balletbo@collabora.com>,
        architt@codeaurora.org, inki.dae@samsung.com,
        thierry.reding@gmail.com, hjc@rock-chips.com,
        seanpaul@chromium.org, airlied@linux.ie, tfiga@chromium.org,
        heiko@sntech.de
Cc:     dri-devel@lists.freedesktop.org, dianders@chromium.org,
        ykk@rock-chips.com, kernel@collabora.com, m.szyprowski@samsung.com,
        linux-samsung-soc@vger.kernel.org, jy0922.shim@samsung.com,
        rydberg@bitmath.org, krzk@kernel.org,
        linux-rockchip@lists.infradead.org, kgene@kernel.org,
        linux-input@vger.kernel.org, orjan.eide@arm.com,
        wxt@rock-chips.com, jeffy.chen@rock-chips.com,
        linux-arm-kernel@lists.infradead.org, mark.yao@rock-chips.com,
        wzz@rock-chips.com, hl@rock-chips.com, jingoohan1@gmail.com,
        sw0312.kim@samsung.com, linux-kernel@vger.kernel.org,
        kyungmin.park@samsung.com, Laurent.pinchart@ideasonboard.com,
        kuankuan.y@gmail.com, hshi@chromium.org
From:   Andrzej Hajda <a.hajda@samsung.com>
Message-ID: <a9e436f8-34ec-123a-9c69-efabb4576bea@samsung.com>
Date:   Mon, 16 Apr 2018 11:57:15 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
        Thunderbird/52.7.0
MIME-Version: 1.0
In-Reply-To: <20180405095000.9756-29-enric.balletbo@collabora.com>
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20180405095000.9756-1-enric.balletbo@collabora.com>
        <CGME20180405095117epcas2p2093448b49aab2abd6684bf689f8888d5@epcas2p2.samsung.com>
        <20180405095000.9756-29-enric.balletbo@collabora.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> It looks like the driver subsystem detaches devices from power domains
> at shutdown without consent of the drivers.

It looks bit strange. Could you elaborate more on it. Could you show the
code performing the detach?


Regards
Andrzej


>  This means that we might have
> our power domain turned off behind our back and the only way to avoid
> problems is to stop doing any hardware programming at some point before
> the power is cut. A reboot notifier, despite being a misnomer and
> handling shutdowns as well, is a good place to do it.
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
>  drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index e7e16d92d5a1..1bf5cba9a64d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/input.h>
> +#include <linux/reboot.h>
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
> @@ -33,6 +34,7 @@ struct psr_drv {
>  	struct delayed_work	flush_work;
>  	struct work_struct	disable_work;
>  
> +	struct notifier_block	reboot_nb;
>  	struct input_handler    input_handler;
>  
>  	int (*set)(struct drm_encoder *encoder, bool enable);
> @@ -309,6 +311,24 @@ static const struct input_device_id psr_ids[] = {
>  	{ },
>  };
>  
> +static int rockchip_drm_psr_reboot_notifier(struct notifier_block *nb,
> +					    unsigned long action, void *data)
> +{
> +	struct psr_drv *psr = container_of(nb, struct psr_drv, reboot_nb);
> +
> +	/*
> +	 * It looks like the driver subsystem detaches devices from power
> +	 * domains at shutdown without consent of the drivers. This means
> +	 * that we might have our power domain turned off behind our back
> +	 * and the only way to avoid problems is to stop doing any hardware
> +	 * programming after this point, which is achieved by the unbalanced
> +	 * call below.
> +	 */
> +	rockchip_drm_psr_inhibit_get(psr->encoder);
> +
> +	return 0;
> +}
> +
>  /**
>   * rockchip_drm_psr_register - register encoder to psr driver
>   * @encoder: encoder that obtain the PSR function
> @@ -361,6 +381,9 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
>  	if (error)
>  		goto err1;
>  
> +	psr->reboot_nb.notifier_call = rockchip_drm_psr_reboot_notifier;
> +	register_reboot_notifier(&psr->reboot_nb);
> +
>  	mutex_lock(&drm_drv->psr_list_lock);
>  	list_add_tail(&psr->list, &drm_drv->psr_list);
>  	mutex_unlock(&drm_drv->psr_list_lock);
> @@ -403,6 +426,7 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder)
>  			WARN_ON(psr->inhibit_count != 1);
>  
>  			list_del(&psr->list);
> +			unregister_reboot_notifier(&psr->reboot_nb);
>  			input_unregister_handler(&psr->input_handler);
>  			kfree(psr->input_handler.name);
>  			kfree(psr);