Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: "Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Mika Westerberg" <mika.westerberg@linux.intel.com>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	"open list:DRM PANEL DRIVERS" <dri-devel@lists.freedesktop.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"ACPI Devel Maling List" <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH 1/2] pinctrl: baytrail: Add GPIO lookup and pinctrl-map for i915 DSI panel ctrl
Date: Mon, 2 Dec 2019 12:39:45 +0100
Message-ID: <CACRpkda9jLpGaQTVco4QyXQKPs3ZODbRb58fPfcAodnxR_s4zA@mail.gmail.com> (raw)
In-Reply-To: <20191129185836.2789-2-hdegoede@redhat.com>

Hi Hans!

Thanks for your patch!

On Fri, Nov 29, 2019 at 7:58 PM Hans de Goede <hdegoede@redhat.com> wrote:

> On Bay Trail devices the MIPI power on/off sequences for DSI LCD panels
> do not control the LCD panel and backlight GPIOs. So far 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 adds GPIO lookups and a pinctrl-map which the i915 driver can
> use to get the panel- and backlight-enable GPIOs and to mux the PWM0 pin
> to the PWM controller.
>
> Note it may seem a bit weird to add a pinctrl-map for the i915 driver,
> so that it can set the PWM0 pinmux. Doing this from the LPSS PWM driver
> would be more logical. But the only thing telling us that the pin should
> definitely be muxed to the PWM controller is the VBT to which the PWM
> driver does not have access.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
(...)

The code setting up the pinctrl map and the GPIO machine
descriptor essentially looks good to me.

This looks a bit weird:

+       if (soc_data == &byt_score_soc_data) {

Can you do this with explicit platform ID string
comparison instead?

if (!strcmp(soc_data->uid, BYT_SCORE_ACPI_UID)) { ...

It seems more appropriate to me.

What is puzzling is the placement inside the pinctrl driver:
normally the thing that spawns the devices on the platform
such as the "byt_gpio" should register this table too.

However I see that the device comes from the ACPI match.

Two questions:

- Is this one of those cases where ACPI "should have
  thought of this" (a common phrase) and we have to mop
  up the mess in the kernel because ACPI didn't and now
  we have to quirk around it?

- Can we in that case handle this with a "boardfile" of
  quirks under say drivers/platform where we register
  some extra stuff rather than inside the pinctrl driver?

It sort of connects to the other review comments where
I feel we should be spawning gpio backlight devices from
somewhere.

I understand those things may be a bit big, if the intel
pinctrl maintainers are fine with this solution I am fine
with it too, it's not like it is the biggest deal, I am just worried
that we might be stockpiling these quirks all over the place
while they should perhaps be centralized.

Yours,
Linus Walleij

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-29 18:58 [PATCH 0/2] drm/i915/vlv_dsi: Control panel and backlight enable GPIOs on BYT Hans de Goede
2019-11-29 18:58 ` [PATCH 1/2] pinctrl: baytrail: Add GPIO lookup and pinctrl-map for i915 DSI panel ctrl Hans de Goede
2019-12-02 11:39   ` Linus Walleij [this message]
2019-12-02 13:09   ` Andy Shevchenko
2019-11-29 18:58 ` [PATCH 2/2] drm/i915/vlv_dsi: Control panel and backlight enable GPIOs on BYT Hans de Goede
2019-12-02 11:21   ` Linus Walleij
2019-12-02 11:53     ` Jani Nikula
2019-12-02 15:49       ` Hans de Goede
2019-12-11  0:24         ` Linus Walleij
2019-11-29 20:07 ` [PATCH 0/2] " Andy Shevchenko

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACRpkda9jLpGaQTVco4QyXQKPs3ZODbRb58fPfcAodnxR_s4zA@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git