From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B55C7C43603 for ; Mon, 16 Dec 2019 14:28:16 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 891F22067C for ; Mon, 16 Dec 2019 14:28:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="aGSyxsPj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 891F22067C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1FB616E5A0; Mon, 16 Dec 2019 14:28:16 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 662936E5A0 for ; Mon, 16 Dec 2019 14:28:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1576506494; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=noMkUix3jvf1eemRUtfdt9onAMruxoWmSHOOhy4r3RM=; b=aGSyxsPjpVajNejciLcJlPPQNe1iQ+d86WiPkSMv9ARBIAvazeqdyyKS6dDbxWbTf3G9Im ZfaLtdZ3HABLV9+8ZczN8T8ZE9HXVzji1jTr8jJS6rOgkX6WvjEOuH8SDIbx+dKhlyc6Ew hBvBidVXXqEzkB7U9uZj7vNLV6iiAA0= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-256-er71w6GlMA6G2vNGpH0t7g-1; Mon, 16 Dec 2019 09:28:12 -0500 Received: by mail-wm1-f72.google.com with SMTP id p2so1111055wma.3 for ; Mon, 16 Dec 2019 06:28:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=mqnAaSxh0pSKzbiLQ7KvpBpDD8sO/UPH/8UcK0z/vf8=; b=ZE6pzGBdMaxUdrtpthvGhu02kZOQjTEIvsXFUOSn/GuSPUuxx5WtBBvaRABFUQHt30 hcfpFuasng4rsCviJKNa8Y6STdwvEQK81JE0iY2jcwghHqMgdCa+N9ik3tfu2mV1m3BF Wn6dgHtkCsjEMeQEa6GdSx5YWwcRMAJtZq31M3Sn8F7i/QKHKlFTsn9fQj7TdjRusSQD UZjVdEfwYUx/2tjBvmxXIVX6drr8DQGB+2Y9SaGGXKwva85XMTU/xF/3mE7JadhHzJ4s DN5jIif2gcQeYucRWbcDWbrAKFnTZ4r2sCFTWU33yF+o4+y8RMUVsIv1u8niENWtDKq3 ldaQ== X-Gm-Message-State: APjAAAUiFgCbf7T/vPNdySSqxgGuZ4dt6cGGcoWAxT4MB5j+zscRAxQO 9/FJLbJDj4CcZVhVICSNdlNfp2FQKxOVX9rMX621ctMie44m7VPyLMc6+6kcTc7UTnv2KyrGjSF g7dvDn1Rtea4neeELejVtvPH5ASft X-Received: by 2002:a7b:c216:: with SMTP id x22mr29355327wmi.51.1576506491407; Mon, 16 Dec 2019 06:28:11 -0800 (PST) X-Google-Smtp-Source: APXvYqz1cdzONxv/4mP1xQlKXrsuF/DPqXX835uoZSpIhEWRShDwrAJ8iqhJqjAy/Yiqp/DSLPbvfQ== X-Received: by 2002:a7b:c216:: with SMTP id x22mr29355302wmi.51.1576506491081; Mon, 16 Dec 2019 06:28:11 -0800 (PST) Received: from shalem.localdomain (2001-1c00-0c0c-fe00-7e79-4dac-39d0-9c14.cable.dynamic.v6.ziggo.nl. [2001:1c00:c0c:fe00:7e79:4dac:39d0:9c14]) by smtp.gmail.com with ESMTPSA id r68sm16146106wmr.43.2019.12.16.06.28.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Dec 2019 06:28:10 -0800 (PST) To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= References: <20191215163810.52356-1-hdegoede@redhat.com> <20191215163810.52356-6-hdegoede@redhat.com> <20191216140427.GT1208@intel.com> From: Hans de Goede Message-ID: <910ef405-39df-8c58-48cb-d3ee407bd60d@redhat.com> Date: Mon, 16 Dec 2019 15:28:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <20191216140427.GT1208@intel.com> Content-Language: en-US X-MC-Unique: er71w6GlMA6G2vNGpH0t7g-1 X-Mimecast-Spam-Score: 0 Subject: Re: [Intel-gfx] [PATCH 5/5] drm/i915/dsi: Control panel and backlight enable GPIOs on BYT X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linus Walleij , linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, dri-devel@lists.freedesktop.org, Lee Jones , intel-gfx Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="windows-1252"; Format="flowed" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Hi, On 16-12-2019 15:04, Ville Syrj=E4l=E4 wrote: > On Sun, Dec 15, 2019 at 05:38:10PM +0100, Hans de Goede wrote: >> On Bay Trail devices the MIPI power on/off sequences for DSI LCD panels >> do not control the LCD panel- and backlight-enable GPIOs. So far, when >> the VBT indicates we should use the SoC for backlight control, we have >> been relying on these GPIOs being configured as output and driven high by >> the Video BIOS (GOP) when it initializes the panel. >> >> This does not work when the device is booted with a HDMI monitor connect= ed >> as then the GOP will initialize the HDMI instead of the panel, leaving t= he >> panel black, even though the i915 driver tries to output an image to it. >> >> Likewise on some device-models when the GOP does not initialize the DSI >> panel it also leaves the mux of the PWM0 pin in generic GPIO mode instead >> of muxing it to the PWM controller. >> >> This commit makes the DSI code control the SoC GPIOs for panel- and >> backlight-enable on BYT, when the VBT indicates the SoC should be used >> >> for backlight control. It also ensures that the PWM0 pin is muxed to the >> PWM controller in this case. >> >> This fixes the LCD panel not lighting up on various devices when booted >> with a HDMI monitor connected. This has been tested to fix this on the >> following devices: >> >> Peaq C1010 >> Point of View MOBII TAB-P800W >> Point of View MOBII TAB-P1005W >> Terra Pad 1061 >> Yours Y8W81 >> >> Signed-off-by: Hans de Goede >> --- >> drivers/gpu/drm/i915/display/intel_dsi.h | 3 +- >> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 63 ++++++++++++++++++++ >> 2 files changed, 65 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h b/drivers/gpu/drm/= i915/display/intel_dsi.h >> index 675771ea91aa..7481a5aa3084 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsi.h >> +++ b/drivers/gpu/drm/i915/display/intel_dsi.h >> @@ -45,8 +45,9 @@ struct intel_dsi { >> struct intel_dsi_host *dsi_hosts[I915_MAX_PORTS]; >> intel_wakeref_t io_wakeref[I915_MAX_PORTS]; >> = >> - /* GPIO Desc for CRC based Panel control */ >> + /* GPIO Desc for panel and backlight control */ >> struct gpio_desc *gpio_panel; >> + struct gpio_desc *gpio_backlight; >> = >> struct intel_connector *attached_connector; >> = >> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/= drm/i915/display/intel_dsi_vbt.c >> index 847f04eec2a1..bd007d4f86e2 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c >> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c >> @@ -27,6 +27,8 @@ >> #include >> #include >> #include >> +#include >> +#include >> #include >> = >> #include >> @@ -525,11 +527,15 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi = *intel_dsi, >> { >> if (seq_id =3D=3D MIPI_SEQ_POWER_ON && intel_dsi->gpio_panel) >> gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); >> + if (seq_id =3D=3D MIPI_SEQ_BACKLIGHT_ON && intel_dsi->gpio_backlight) >> + gpiod_set_value_cansleep(intel_dsi->gpio_backlight, 1); >> = >> intel_dsi_vbt_exec(intel_dsi, seq_id); >> = >> if (seq_id =3D=3D MIPI_SEQ_POWER_OFF && intel_dsi->gpio_panel) >> gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0); >> + if (seq_id =3D=3D MIPI_SEQ_BACKLIGHT_OFF && intel_dsi->gpio_backlight) >> + gpiod_set_value_cansleep(intel_dsi->gpio_backlight, 0); >> } >> = >> void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec) >> @@ -688,6 +694,8 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi,= u16 panel_id) >> /* >> * On some BYT/CHT devs some sequences are incomplete and we need to m= anually >> * control some GPIOs. We need to add a GPIO lookup table before we ge= t these. >> + * If the GOP did not initialize the panel (HDMI inserted) we may need = to also >> + * change the pinmux for the SoC's PWM0 pin from GPIO to PWM. >> */ >> static struct gpiod_lookup_table pmic_panel_gpio_table =3D { >> /* Intel GFX is consumer */ >> @@ -699,23 +707,68 @@ static struct gpiod_lookup_table pmic_panel_gpio_t= able =3D { >> }, >> }; >> = >> +static struct gpiod_lookup_table soc_panel_gpio_table =3D { >> + .dev_id =3D "0000:00:02.0", >> + .table =3D { >> + GPIO_LOOKUP("INT33FC:01", 10, "backlight", GPIO_ACTIVE_HIGH), >> + GPIO_LOOKUP("INT33FC:01", 11, "panel", GPIO_ACTIVE_HIGH), >> + { }, > = > Some kind of indent fail here. Yeah, this was intentional in the previous revision because of the 80 char limit, but this indent hack is no longer necessary, fixed for the v2 of this set which I'm preparing. >> + }, >> +}; >> + >> +static const struct pinctrl_map soc_pwm_pinctrl_map[] =3D { >> + PIN_MAP_MUX_GROUP("0000:00:02.0", "soc_pwm0", "INT33FC:00", >> + "pwm0_grp", "pwm"), >> +}; >> + >> void intel_dsi_vbt_gpio_init(struct intel_dsi *intel_dsi, bool panel_i= s_on) >> { >> struct drm_device *dev =3D intel_dsi->base.base.dev; >> struct drm_i915_private *dev_priv =3D to_i915(dev); >> struct mipi_config *mipi_config =3D dev_priv->vbt.dsi.config; >> enum gpiod_flags flags =3D panel_is_on ? GPIOD_OUT_HIGH : GPIOD_OUT_L= OW; >> + bool want_backlight_gpio =3D false; >> + bool want_panel_gpio =3D false; >> + struct pinctrl *pinctrl; >> + int ret; >> = >> if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && >> (mipi_config->pwm_blc =3D=3D PPS_BLC_PMIC)) { >> gpiod_add_lookup_table(&pmic_panel_gpio_table); >> + want_panel_gpio =3D true; >> + } >> + >> + if (IS_VALLEYVIEW(dev_priv) && mipi_config->pwm_blc =3D=3D PPS_BLC_SOC= ) { >> + gpiod_add_lookup_table(&soc_panel_gpio_table); >> + want_panel_gpio =3D true; >> + want_backlight_gpio =3D true; >> = >> + /* Ensure PWM0 pin is muxed as PWM instead of GPIO */ >> + ret =3D pinctrl_register_mappings(soc_pwm_pinctrl_map, 1); > = > ARRAY_SIZE()? Ack, will fix for v2. > = >> + if (ret) >> + DRM_ERROR("Failed to register pwm0 pinmux mapping\n"); >> + >> + pinctrl =3D devm_pinctrl_get_select(dev->dev, "soc_pwm0"); >> + if (IS_ERR(pinctrl)) >> + DRM_ERROR("Failed to set pinmux to PWM\n"); >> + } >> + >> + if (want_panel_gpio) { >> intel_dsi->gpio_panel =3D gpiod_get(dev->dev, "panel", flags); >> if (IS_ERR(intel_dsi->gpio_panel)) { >> DRM_ERROR("Failed to own gpio for panel control\n"); >> intel_dsi->gpio_panel =3D NULL; >> } >> } >> + >> + if (want_backlight_gpio) { >> + intel_dsi->gpio_backlight =3D >> + gpiod_get(dev->dev, "backlight", flags); >> + if (IS_ERR(intel_dsi->gpio_backlight)) { >> + DRM_ERROR("Failed to own gpio for backlight control\n"); >> + intel_dsi->gpio_backlight =3D NULL; >> + } >> + } >> } >> = >> void intel_dsi_vbt_gpio_cleanup(struct intel_dsi *intel_dsi) >> @@ -729,7 +782,17 @@ void intel_dsi_vbt_gpio_cleanup(struct intel_dsi *i= ntel_dsi) >> intel_dsi->gpio_panel =3D NULL; >> } >> = >> + if (intel_dsi->gpio_backlight) { >> + gpiod_put(intel_dsi->gpio_backlight); >> + intel_dsi->gpio_backlight =3D NULL; >> + } >> + >> if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && >> (mipi_config->pwm_blc =3D=3D PPS_BLC_PMIC)) >> gpiod_remove_lookup_table(&pmic_panel_gpio_table); >> + >> + if (IS_VALLEYVIEW(dev_priv) && mipi_config->pwm_blc =3D=3D PPS_BLC_SOC= ) { > = > Slightly annoying to have these checks duplicated. Might be cleaner to > have a few helpers that return the correct tables and just use those in > both init and cleanup. OTOH those want_*_gpio flags and the pwm stuff is > would still be a sticking point I suppose. So maybe not cleaner in the > end after all. So I tried adding a helper for the if condition, since as you mention just returning the right table is not really helpful: static bool intel_dsi_vbt_use_pmic_backlight_ctl(struct intel_dsi *intel_ds= i) { struct drm_device *dev =3D intel_dsi->base.base.dev; struct drm_i915_private *dev_priv =3D to_i915(dev); struct mipi_config *mipi_config =3D dev_priv->vbt.dsi.config; if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && (mipi_config->pwm_blc =3D=3D PPS_BLC_PMIC)) { return (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && mipi_config->pwm_blc =3D=3D PPS_BLC_PMIC; } And copy and paste that for the PPS_BLC_SOC case. The result does not look a lot better then the original, worse actually IMHO, so I'm going to keep this as is for v2. > Looks all right to me: > Reviewed-by: Ville Syrj=E4l=E4 Thanks. Regards, Hans > = > = >> + pinctrl_unregister_mappings(soc_pwm_pinctrl_map); >> + gpiod_remove_lookup_table(&soc_panel_gpio_table); >> + } >> } >> -- = >> 2.23.0 > = _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx