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;
Date:   Fri, 26 Jul 2019 12:25:29 +0200
From:   Sam Ravnborg <sam@ravnborg.org>
To:     Guido =?iso-8859-1?Q?G=FCnther?= <agx@sigxcpu.org>
Cc:     Purism Kernel Team <kernel@puri.sm>,
        Thierry Reding <thierry.reding@gmail.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] drm/panel: jh057n00900: Move dsi init sequence to
 prepare
Message-ID: <20190726102529.GB15658@ravnborg.org>
References: <cover.1564132646.git.agx@sigxcpu.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <cover.1564132646.git.agx@sigxcpu.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Guido.

On Fri, Jul 26, 2019 at 11:21:40AM +0200, Guido G?nther wrote:
> If the panel is wrapped in a panel_bridge it gets prepar()ed before the
> upstream DSI bridge which can cause hangs (e.g. with imx-nwl since clocks
> are not enabled yet). To avoid this move the panel's first DSI access to
> enable() so the upstream bridge can prepare the DSI host controller in
> it's pre_enable().
> 
> The second patch makes the disable() call symmetric to the above and the third
> one just eases debugging.
> 
> Guido G?nther (3):
>   drm/panel: jh057n00900: Move panel DSI init to enable()
>   drm/panel: jh057n00900: Move mipi_dsi_dcs_set_display_off to disable()
>   drm/panel: jh057n00900: Print error code on all DRM_DEV_ERROR()s

Patch 1 + 3 are both:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

See comment on patch 2.

While you are touching this driver can you make an extra patch?

Today the driver calls the internal prepare,enable,disable,unprepare
functions.
The right way to do it is to use the
drm_panel_(prepare,enable,disable,unprepare) variants.

The benefit is that we can move a little logic to these functions
and the drivers will then benefit from this.

Two things I have in my local queue:
- Move bool for prepared/enabled
  (to protect that we do not prepare/enable twice)
- backlight support

This driver will benefit form both and this little modification will
make it simpler to introduce.
I can also prepare the patch if you prefer.

	Sam