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 connected
> as then the GOP will initialize the HDMI instead of the panel, leaving the
> 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 <hdegoede@xxxxxxxxxx>
> ---
> 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 <linux/gpio/consumer.h>
> #include <linux/gpio/machine.h>
> #include <linux/mfd/intel_soc_pmic.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/pinctrl/machine.h>
> #include <linux/slab.h>
>
> #include <asm/intel-mid.h>
> @@ -525,11 +527,15 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
> {
> if (seq_id == MIPI_SEQ_POWER_ON && intel_dsi->gpio_panel)
> gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> + if (seq_id == 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 == MIPI_SEQ_POWER_OFF && intel_dsi->gpio_panel)
> gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
> + if (seq_id == 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 manually
> * control some GPIOs. We need to add a GPIO lookup table before we get 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 = {
> /* Intel GFX is consumer */
> @@ -699,23 +707,68 @@ static struct gpiod_lookup_table pmic_panel_gpio_table = {
> },
> };
>
> +static struct gpiod_lookup_table soc_panel_gpio_table = {
> + .dev_id = "0000:00:02.0",
> + .table = {
> + GPIO_LOOKUP("INT33FC:01", 10, "backlight", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("INT33FC:01", 11, "panel", GPIO_ACTIVE_HIGH),
> + { },
Some kind of indent fail here.
> + },
> +};
> +
> +static const struct pinctrl_map soc_pwm_pinctrl_map[] = {
> + 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_is_on)
> {
> struct drm_device *dev = intel_dsi->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
> enum gpiod_flags flags = panel_is_on ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
> + bool want_backlight_gpio = false;
> + bool want_panel_gpio = false;
> + struct pinctrl *pinctrl;
> + int ret;
>
> if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
> (mipi_config->pwm_blc == PPS_BLC_PMIC)) {
> gpiod_add_lookup_table(&pmic_panel_gpio_table);
> + want_panel_gpio = true;
> + }
> +
> + if (IS_VALLEYVIEW(dev_priv) && mipi_config->pwm_blc == PPS_BLC_SOC) {
> + gpiod_add_lookup_table(&soc_panel_gpio_table);
> + want_panel_gpio = true;
> + want_backlight_gpio = true;
>
> + /* Ensure PWM0 pin is muxed as PWM instead of GPIO */
> + ret = pinctrl_register_mappings(soc_pwm_pinctrl_map, 1);
ARRAY_SIZE()?
> + if (ret)
> + DRM_ERROR("Failed to register pwm0 pinmux mapping\n");
> +
> + pinctrl = 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 = 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 = NULL;
> }
> }
> +
> + if (want_backlight_gpio) {
> + intel_dsi->gpio_backlight =
> + 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 = 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 *intel_dsi)
> intel_dsi->gpio_panel = NULL;
> }
>
> + if (intel_dsi->gpio_backlight) {
> + gpiod_put(intel_dsi->gpio_backlight);
> + intel_dsi->gpio_backlight = NULL;
> + }
> +
> if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
> (mipi_config->pwm_blc == PPS_BLC_PMIC))
> gpiod_remove_lookup_table(&pmic_panel_gpio_table);
> +
> + if (IS_VALLEYVIEW(dev_priv) && mipi_config->pwm_blc == 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.
Looks all right to me:
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> + pinctrl_unregister_mappings(soc_pwm_pinctrl_map);
> + gpiod_remove_lookup_table(&soc_panel_gpio_table);
> + }
> }
> --
> 2.23.0
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
