Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1591009402-681-1-git-send-email-mkrishn@codeaurora.org>
 <CACvgo50eb5_jp_6B5tkV9cX_X2_y2Xnavu+wvUUhHN5FsV9hiw@mail.gmail.com>
 <cd61dd742e73b89794fc1b812d9fdcd9@codeaurora.org> <CACvgo50b+m2+onak=AZfgihkBXEP9POjMR52087v==-puLdkQQ@mail.gmail.com>
 <8742ac6fbd498fdc22dcd469c3a2d52a@codeaurora.org>
In-Reply-To: <8742ac6fbd498fdc22dcd469c3a2d52a@codeaurora.org>
From:   Emil Velikov <emil.l.velikov@gmail.com>
Date:   Fri, 5 Jun 2020 15:11:24 +0100
Message-ID: <CACvgo52Rz+QzA4k7ttg6Gh124fZBAfaX0nrPpaMTECvr_8D2zw@mail.gmail.com>
Subject: Re: [v2] drm/msm: add shutdown support for display platform_driver
To:     Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Cc:     Krishna Manikandan <mkrishn@codeaurora.org>,
        ML dri-devel <dri-devel@lists.freedesktop.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        freedreno@lists.freedesktop.org,
        devicetree <devicetree@vger.kernel.org>,
        "Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>,
        Sean Paul <seanpaul@chromium.org>, kalyan_t@codeaurora.org,
        "Kristian H . Kristensen" <hoegsberg@chromium.org>,
        mka@chromium.org, devicetree-owner@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 2 Jun 2020 at 17:10, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Hi Emil,
>
> On 2020-06-02 21:09, Emil Velikov wrote:
> > On Tue, 2 Jun 2020 at 15:49, Sai Prakash Ranjan
> > <saiprakash.ranjan@codeaurora.org> wrote:
> >>
> >> Hi Emil,
> >>
> >> On 2020-06-02 19:43, Emil Velikov wrote:
> >> > Hi Krishna,
> >> >
> >> > On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan
> >> > <mkrishn@codeaurora.org> wrote:
> >> >>
> >> >> Define shutdown callback for display drm driver,
> >> >> so as to disable all the CRTCS when shutdown
> >> >> notification is received by the driver.
> >> >>
> >> >> This change will turn off the timing engine so
> >> >> that no display transactions are requested
> >> >> while mmu translations are getting disabled
> >> >> during reboot sequence.
> >> >>
> >> >> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
> >> >>
> >> > AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
> >> > msm_drm_ops::unbind.
> >> >
> >> > Are you saying that unbind never triggers? If so, then we should
> >> > really fix that instead, since this patch seems more like a
> >> > workaround.
> >> >
> >>
> >> Which path do you suppose that the unbind should be called from,
> >> remove
> >> callback? Here we are talking about the drivers which are builtin,
> >> where
> >> remove callbacks are not called from the driver core during
> >> reboot/shutdown,
> >> instead shutdown callbacks are called which needs to be defined in
> >> order
> >> to
> >> trigger unbind. So AFAICS there is nothing to be fixed.
> >>
> > Interesting - in drm effectively only drm panels implement .shutdown.
> > So my naive assumption was that .remove was used implicitly by core,
> > as part of the shutdown process. Yet that's not the case, so it seems
> > that many drivers could use some fixes.
> >
> > Then again, that's an existing problem which is irrelevant for msm.
> > -Emil
>
> To give more context, we are actually targeting the clients/consumers
> of SMMU/IOMMU here because we have to make sure that before the supplier
> (SMMU) shuts down, its consumers/clients need to be shutdown properly.
> Now the ordering of this is taken care in the SMMU driver via
> device_link
> which makes sure that consumer shutdown callbacks are called first, but
> we
> need to define shutdown callbacks for all its consumers to make sure we
> actually shutdown the clients or else it would invite the crashes during
> reboot
> which in this case was seen for display.
>
Thank you very much for the extra details. I think other DRM drivers
will be safe as-is.

-Emil