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;
Date:   Mon, 6 Nov 2023 00:54:38 +0200
From:   Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To:     Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc:     Aradhya Bhatia <a-bhatia1@ti.com>,
        Devarsh Thakkar <devarsht@ti.com>,
        Jyri Sarha <jyri.sarha@iki.fi>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 04/10] drm/tidss: Move reset to the end of dispc_init()
Message-ID: <20231105225438.GB15635@pendragon.ideasonboard.com>
References: <20231101-tidss-probe-v1-0-45149e0f9415@ideasonboard.com>
 <20231101-tidss-probe-v1-4-45149e0f9415@ideasonboard.com>
 <20231101135749.GT12764@pendragon.ideasonboard.com>
 <71056358-f91e-4a88-a979-33bfe157289a@ideasonboard.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <71056358-f91e-4a88-a979-33bfe157289a@ideasonboard.com>
Precedence: bulk

On Thu, Nov 02, 2023 at 08:40:10AM +0200, Tomi Valkeinen wrote:
> On 01/11/2023 15:57, Laurent Pinchart wrote:
> > On Wed, Nov 01, 2023 at 11:17:41AM +0200, Tomi Valkeinen wrote:
> >> We do a DSS reset in the middle of the dispc_init(). While that happens
> >> to work now, we should really make sure that e..g the fclk, which is
> >> acquired only later in the function, is enabled when doing a reset. This
> >> will be handled in a later patch, but for now, let's move the
> >> dispc_softreset() call to the end of dispc_init(), which is a sensible
> >> place for it anyway.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > But do I understand correctly that the device isn't powered up at this
> > point ? That seems problematic.
> 
> Indeed. It's fixed later in this series.
> 
> > I'm also not sure why we need to reset the device at probe time.
> 
> That's the usual place to do a reset, to make sure the HW is in a known 
> state, is it not? Where would you place it?

The first time the device is used, or possibly every time it is resumed
? It seems that you're resuming it at probe time for the only reason
that you want to then reset it. Resuming it at probe could get entirely
skipped.

-- 
Regards,

Laurent Pinchart