From mboxrd@z Thu Jan 1 00:00:00 1970 From: Emil Velikov Subject: Re: [PATCH v1 14/16] drm/panel: call prepare/enable only once Date: Mon, 5 Aug 2019 14:15:55 +0100 Message-ID: <20190805131555.GA5005@arch-x1c3> References: <20190804201637.1240-1-sam@ravnborg.org> <20190804201637.1240-15-sam@ravnborg.org> <20190805105928.GI29747@pendragon.ideasonboard.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20190805105928.GI29747@pendragon.ideasonboard.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Laurent Pinchart Cc: Neil Armstrong , David Airlie , dri-devel@lists.freedesktop.org, Thierry Reding , Sam Ravnborg , Marek Vasut , linux-samsung-soc@vger.kernel.org, Sean Paul , Allison Randal , Krzysztof Kozlowski , Jonathan Hunter , Maxime Ripard , Kukjin Kim , NXP Linux Team , Jonas Karlman , Alison Wang , Gwan-gyeong Mun , Alexios Zavras , Laurent Pinchart , linux-tegra@vger.kernel.org, Thomas Gleixner , Vincent Abriou , linux-arm-kernel@lists.infradead.org List-Id: linux-tegra@vger.kernel.org On 2019/08/05, Laurent Pinchart wrote: > Hi Sam, > > Thank you for the patch. > > On Sun, Aug 04, 2019 at 10:16:35PM +0200, Sam Ravnborg wrote: > > Many panel drivers duplicate logic to prevent prepare to be called > > for a panel that is already prepared. > > Likewise for enable. > > > > Implement this logic in drm_panel so the individual drivers > > no longer needs this. > > A panel is considered prepared/enabled only if the prepare/enable call > > succeeds. > > For disable/unprepare it is unconditionally considered > > disabled/unprepared. > > > > This allows calls to prepare/enable again, even if there were > > some issue disabling a regulator or similar during disable/unprepare. > > Is this the right place to handle this ? Shouldn't the upper layers > ensure than enable/disable and prepare/unprepare are correcty balanced, > and not called multiple times ? Adding enabled and prepared state to > drm_panel not only doesn't align well with atomic state handling, but > also would hide issues in upper layers that should really be fixed > there. > Fully agreed. Mistakes happen - hiding them, by returning "success" does not sound like a wise approach. HTH Emil