[PATCH] drm/panel: boe-tv101wum-nl6: Ensure DSI writes succeed during disable
Thomas Zimmermann
tzimmermann at suse.de
Wed Feb 1 03:02:31 PST 2023
Am 31.01.23 um 22:27 schrieb Doug Anderson:
> Hi,
>
> On Thu, Jan 26, 2023 at 4:52 PM Doug Anderson <dianders at chromium.org> wrote:
>>
>> Hi,
>>
>> On Wed, Jan 18, 2023 at 1:34 PM Doug Anderson <dianders at chromium.org> wrote:
>>>
>>> Hi,
>>>
>>> On Thu, Jan 5, 2023 at 7:01 PM Stephen Boyd <swboyd at chromium.org> wrote:
>>>>
>>>> The unprepare sequence has started to fail after moving to panel bridge
>>>> code in the msm drm driver (commit 007ac0262b0d ("drm/msm/dsi: switch to
>>>> DRM_PANEL_BRIDGE")). You'll see messages like this in the kernel logs:
>>>>
>>>> panel-boe-tv101wum-nl6 ae94000.dsi.0: failed to set panel off: -22
>>>>
>>>> This is because boe_panel_enter_sleep_mode() needs an operating DSI link
>>>> to set the panel into sleep mode. Performing those writes in the
>>>> unprepare phase of bridge ops is too late, because the link has already
>>>> been torn down by the DSI controller in post_disable, i.e. the PHY has
>>>> been disabled, etc. See dsi_mgr_bridge_post_disable() for more details
>>>> on the DSI .
>>>>
>>>> Split the unprepare function into a disable part and an unprepare part.
>>>> For now, just the DSI writes to enter sleep mode are put in the disable
>>>> function. This fixes the panel off routine and keeps the panel happy.
>>>>
>>>> My Wormdingler has an integrated touchscreen that stops responding to
>>>> touch if the panel is only half disabled too. This patch fixes it. And
>>>> finally, this saves power when the screen is off because without this
>>>> fix the regulators for the panel are left enabled when nothing is being
>>>> displayed on the screen.
>>>>
>>>> Fixes: 007ac0262b0d ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE")
>>>> Fixes: a869b9db7adf ("drm/panel: support for boe tv101wum-nl6 wuxga dsi video mode panel")
>>>> Cc: yangcong <yangcong5 at huaqin.corp-partner.google.com>
>>>> Cc: Douglas Anderson <dianders at chromium.org>
>>>> Cc: Jitao Shi <jitao.shi at mediatek.com>
>>>> Cc: Sam Ravnborg <sam at ravnborg.org>
>>>> Cc: Rob Clark <robdclark at chromium.org>
>>>> Cc: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>> Signed-off-by: Stephen Boyd <swboyd at chromium.org>
>>>> ---
>>>> drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 16 ++++++++++++----
>>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
>>>> index 857a2f0420d7..c924f1124ebc 100644
>>>> --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
>>>> +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
>>>> @@ -1193,14 +1193,11 @@ static int boe_panel_enter_sleep_mode(struct boe_panel *boe)
>>>> return 0;
>>>> }
>>>>
>>>> -static int boe_panel_unprepare(struct drm_panel *panel)
>>>> +static int boe_panel_disable(struct drm_panel *panel)
>>>> {
>>>> struct boe_panel *boe = to_boe_panel(panel);
>>>> int ret;
>>>>
>>>> - if (!boe->prepared)
>>>> - return 0;
>>>> -
>>>> ret = boe_panel_enter_sleep_mode(boe);
>>>> if (ret < 0) {
>>>> dev_err(panel->dev, "failed to set panel off: %d\n", ret);
>>>> @@ -1209,6 +1206,16 @@ static int boe_panel_unprepare(struct drm_panel *panel)
>>>>
>>>> msleep(150);
>>>>
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int boe_panel_unprepare(struct drm_panel *panel)
>>>> +{
>>>> + struct boe_panel *boe = to_boe_panel(panel);
>>>> +
>>>> + if (!boe->prepared)
>>>> + return 0;
>>>> +
>>>> if (boe->desc->discharge_on_disable) {
>>>> regulator_disable(boe->avee);
>>>> regulator_disable(boe->avdd);
>>>> @@ -1528,6 +1535,7 @@ static enum drm_panel_orientation boe_panel_get_orientation(struct drm_panel *pa
>>>> }
>>>>
>>>> static const struct drm_panel_funcs boe_panel_funcs = {
>>>> + .disable = boe_panel_disable,
>>>> .unprepare = boe_panel_unprepare,
>>>> .prepare = boe_panel_prepare,
>>>> .enable = boe_panel_enable,
>>>
>>> As mentioned by Stephen, my initial reaction was that this felt
>>> asymmetric. We were moving some stuff from unprepare() to disable()
>>> and it felt like that would mean we would also need to move something
>>> from prepare() to enable. Initially I thought maybe that "something"
>>> was all of boe_panel_init_dcs_cmd() but I guess that didn't work.
>>>
>>> I don't truly have a reason that this _has_ to be symmetric. I was
>>> initially worried that there might be some place where we call
>>> pre_enable(), then never call enable() / disable(), and then call
>>> post_disable(). That could have us in a bad state because we'd never
>>> enter sleep mode / turn the display off. However (as I think I've
>>> discovered before and just forgot), I don't think this is possible
>>> because we always call pre-enable() and enable() together. Also, as
>>> mentioned by Sam, we're about to fully shut the panel's power off so
>>> (unless it's on a shared rail) it probably doesn't really matter.
>>>
>>> Thus, I'd be OK with:
>>>
>>> Reviewed-by: Douglas Anderson <dianders at chromium.org>
>>>
>>> I'm also happy to land this (adding Cc: stable) to drm-misc-fixes if
>>> nobody has any objections (also happy if someone else wants to land
>>> it). I guess the one worry I have is that somehow this could break
>>> something for one of the other 8 panels that this driver supports (or
>>> it could have bad interactions with the display controller used on a
>>> board with one of these panels?). Maybe we should have "Cc: stable"
>>> off just to give it extra bake time? ...and maybe even push to
>>> drm-misc-next instead of -fixes again to give extra bake time?
>>
>> This thread has gone silent. I'll plan to land the patch in
>> drm-misc-next early next week, maybe Monday, _without_ Ccing stable,
>> so we have the longest possible bake time. If anyone has objections,
>> please yell.
>
> Pushed to drm-misc-next:
I've seen this discussion too late. I have now cherry-picked the patch
into drm-misc-fixes.
>
> c913cd548993 drm/panel: boe-tv101wum-nl6: Ensure DSI writes succeed
> during disable
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20230201/8df76b1f/attachment-0001.sig>
More information about the linux-arm-kernel
mailing list