linux-kernel - Re: [Freedreno] [PATCH] drm/msm/dsi: Delay drm_panel_enable() until dsi_mgr_bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191108234654.GA997@gerhold.net>
Date:   Sat, 9 Nov 2019 00:46:54 +0100
From:   Stephan Gerhold <stephan@...hold.net>
To:     Jeffrey Hugo <jeffrey.l.hugo@...il.com>
Cc:     Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        Jasper Korten <jja2000@...il.com>,
        Hai Li <hali@...eaurora.org>, David Airlie <airlied@...ux.ie>,
        MSM <linux-arm-msm@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "open list:DRM PANEL DRIVERS" <dri-devel@...ts.freedesktop.org>,
        Daniel Vetter <daniel@...ll.ch>,
        freedreno <freedreno@...ts.freedesktop.org>
Subject: Re: [Freedreno] [PATCH] drm/msm/dsi: Delay drm_panel_enable() until
 dsi_mgr_bridge_enable()

On Fri, Nov 08, 2019 at 03:12:28PM -0700, Jeffrey Hugo wrote:
> On Fri, Nov 8, 2019 at 2:29 PM Stephan Gerhold <stephan@...hold.net> wrote:
> >
> > At the moment, the MSM DSI driver calls drm_panel_enable() rather early
> > from the DSI bridge pre_enable() function. At this point, the encoder
> > (e.g. MDP5) is not enabled, so we have not started transmitting
> > video data.
> >
> > However, the drm_panel_funcs documentation states that enable()
> > should be called on the panel *after* video data is being transmitted:
> >
> >   The .prepare() function is typically called before the display controller
> >   starts to transmit video data. [...] After the display controller has
> >   started transmitting video data, it's safe to call the .enable() function.
> >   This will typically enable the backlight to make the image on screen visible.
> >
> > Calling drm_panel_enable() too early causes problems for some panels:
> > The TFT LCD panel used in the Samsung Galaxy Tab A 9.7 (2015) (APQ8016)
> > uses the MIPI_DCS_SET_DISPLAY_BRIGHTNESS command to control
> > backlight/brightness of the screen. The enable sequence is therefore:
> >
> >   drm_panel_enable()
> >     drm_panel_funcs.enable():
> >       backlight_enable()
> >         backlight_ops.update_status():
> >           mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness);
> >
> > The panel seems to silently ignore the MIPI_DCS_SET_DISPLAY_BRIGHTNESS
> > command if it is sent too early. This prevents setting the initial brightness,
> > causing the display to be enabled with minimum brightness instead.
> > Adding various delays in the panel initialization code does not result
> > in any difference.
> >
> > On the other hand, moving drm_panel_enable() to dsi_mgr_bridge_enable()
> > fixes the problem, indicating that the panel requires the video stream
> > to be active before the brightness command is accepted.
> >
> > Therefore: Move drm_panel_enable() to dsi_mgr_bridge_enable() to
> > delay calling it until video data is being transmitted.
> >
> > Move drm_panel_disable() to dsi_mgr_bridge_disable() for similar reasons.
> > (This is not strictly required for the panel affected above...)
> >
> > Tested-by: Jasper Korten <jja2000@...il.com>
> > Signed-off-by: Stephan Gerhold <stephan@...hold.net>
> > ---
> > Since this is a core change I thought it would be better to send this
> > early. I believe Jasper still wants to finish some other changes before
> > submitting the initial device tree for the Samsung Galaxy Tab A 9.7 (2015). ;)
> >
> > I also tested it on msm8916-samsung-a5u-eur, its display is working fine
> > with or without this patch.
> 
> Nack, please.  I was curious so I threw this on the Lenovo Miix 630
> laptop.  I don't get a display back with this patch.  I'll try to
> figure out why, but currently I can't get into the machine.

Thanks for testing the patch! Let's try to figure that out...

I'm a bit confused, but this might be because I'm not very familiar with
the MSM8998 laptops. It does not seem to have display in mainline yet,
so do you have a link to all the patches you are using at the moment?

Judging from the patches I was able to find, the Lenovo Miix 630 is
using a DSI to eDP bridge.
Isn't the panel managed by the bridge driver in that case?

struct msm_dsi contains:
	/*
	 * panel/external_bridge connected to dsi bridge output, only one of the
	 * two can be valid at a time
	 */
	struct drm_panel *panel;
	struct drm_bridge *external_bridge;

So if you have "external_bridge" set in your case, "panel" should be NULL.
I have only moved code that uses msm_dsi->panel, so my patch really
shouldn't make any difference for you.

Am I confusing something here?

Thanks,
Stephan