2019-11-19 15:19:59

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 0/3] drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT

Hi All,

This series needs to be merged through a single tree, to keep things
bisectable. I have even considered just squashing all 3 patches into 1,
but having separate commits seems better, but that does lead to an
intermediate state where the backlight sysfs interface will be broken
(and fixed 2 commits later). See below for some background info.

The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c
are quite small and should not lead to any conflicts, so I believe that
it would be best to merge this entire series through the drm-intel tree.

Lee, may I have your Acked-by for merging the mfd change through the
drm-intel tree?

Rafael, may I have your Acked-by for merging the acpi_lpss change through the
drm-intel tree?

Regards,

Hans

p.s.

The promised background info:

We have this long standing issue where instead of looking in the i915
VBT (Video BIOS Table) to see if we should use the PWM block of the SoC
or of the PMIC to control the backlight of a DSI panel, we rely on
drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c
registering a pwm with the generic name of "pwm_backlight" and then the
i915 panel code does a pwm_get(dev, "pwm_backlight").

We have some heuristics in drivers/acpi/acpi_lpss.c to not register the
lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c
code simply assumes that since there is a PMIC the PMIC PWM block will
be used. Basically we are winging it.

Recently I've learned about 2 different BYT devices:
Point of View MOBII TAB-P800W
Acer Switch 10 SW5-012

Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
PWM controller (and the VBT correctly indicates this), so here our old
heuristics fail.

This series renams the PWM lookups registered by the LPSS /
intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp.
"pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when
to register the lookup. This combined with teaching the i915 panel to call
pwm_get for the right lookup-name depending on the VBT bits resolves this.



2019-11-19 15:20:33

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 3/3] drm/i915: DSI: select correct PWM controller to use based on the VBT

At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
different PWM controllers for controlling the LCD's backlight brightness.
Either the one integrated into the PMIC or the one integrated into the
SoC (the 1st LPSS PWM controller).

So far in the LPSS code on BYT we have skipped registering the LPSS PWM
controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
present, assuming that in this case the PMIC PWM controller will be used.

On CHT we have been relying on only 1 of the 2 PWM controllers being
enabled in the DSDT at the same time; and always registered the lookup.

So far this has been working, but the correct way to determine which PWM
controller needs to be used is by checking a bit in the VBT table and
recently I've learned about 2 different BYT devices:
Point of View MOBII TAB-P800W
Acer Switch 10 SW5-012

Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
PWM controller (and the VBT correctly indicates this), so here our old
heuristics fail.

This commit fixes using the wrong PWM controller on these devices by
calling pwm_get() for the right PWM controller based on the
VBT dsi.config.pwm_blc bit.

Note this is part of a series which contains 2 other patches which renames
the PWM lookup for the 1st SoC/LPSS PWM from "pwm_backlight" to
"pwm_pmic_backlight" and the PWM lookup for the Crystal Cove PMIC PWM
from "pwm_backlight" to "pwm_pmic_backlight".

Signed-off-by: Hans de Goede <[email protected]>
---
drivers/gpu/drm/i915/display/intel_panel.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index bc14e9c0285a..ddcf311d1114 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -1840,13 +1840,22 @@ static int pwm_setup_backlight(struct intel_connector *connector,
enum pipe pipe)
{
struct drm_device *dev = connector->base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_panel *panel = &connector->panel;
+ const char *desc;
int retval;

- /* Get the PWM chip for backlight control */
- panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight");
+ /* Get the right PWM chip for DSI backlight according to VBT */
+ if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
+ panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight");
+ desc = "PMIC";
+ } else {
+ panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight");
+ desc = "SoC";
+ }
+
if (IS_ERR(panel->backlight.pwm)) {
- DRM_ERROR("Failed to own the pwm chip\n");
+ DRM_ERROR("Failed to get the %s PWM chip\n", desc);
panel->backlight.pwm = NULL;
return -ENODEV;
}
@@ -1873,6 +1882,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,
CRC_PMIC_PWM_PERIOD_NS);
panel->backlight.enabled = panel->backlight.level != 0;

+ DRM_INFO("Using %s PWM for LCD backlight control\n", desc);
return 0;
}

--
2.23.0


2019-11-19 15:22:20

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
different PWM controllers for controlling the LCD's backlight brightness.

Either the one integrated into the PMIC or the one integrated into the
SoC (the 1st LPSS PWM controller).

So far in the LPSS code on BYT we have skipped registering the LPSS PWM
controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
present, assuming that in this case the PMIC PWM controller will be used.

On CHT we have been relying on only 1 of the 2 PWM controllers being
enabled in the DSDT at the same time; and always registered the lookup.

So far this has been working, but the correct way to determine which PWM
controller needs to be used is by checking a bit in the VBT table and
recently I've learned about 2 different BYT devices:
Point of View MOBII TAB-P800W
Acer Switch 10 SW5-012

Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
PWM controller (and the VBT correctly indicates this), so here our old
heuristics fail.

Since only the i915 driver has access to the VBT, this commit renames
the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
controller to "pwm_pmic_backlight" so that the i915 driver can do a
pwm_get() for the right controller depending on the VBT bit, instead of
the i915 driver relying on a "pwm_backlight" lookup getting registered
which magically points to the right controller.

Signed-off-by: Hans de Goede <[email protected]>
---
drivers/mfd/intel_soc_pmic_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_core.c b/drivers/mfd/intel_soc_pmic_core.c
index c9f35378d391..47188df3080d 100644
--- a/drivers/mfd/intel_soc_pmic_core.c
+++ b/drivers/mfd/intel_soc_pmic_core.c
@@ -38,7 +38,7 @@ static struct gpiod_lookup_table panel_gpio_table = {

/* PWM consumed by the Intel GFX */
static struct pwm_lookup crc_pwm_lookup[] = {
- PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_backlight", 0, PWM_POLARITY_NORMAL),
+ PWM_LOOKUP("crystal_cove_pwm", 0, "0000:00:02.0", "pwm_pmic_backlight", 0, PWM_POLARITY_NORMAL),
};

static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
--
2.23.0


2019-11-19 15:44:47

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH 0/3] drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT

On Tue, 19 Nov 2019, Hans de Goede <[email protected]> wrote:
> Hi All,
>
> This series needs to be merged through a single tree, to keep things
> bisectable. I have even considered just squashing all 3 patches into 1,
> but having separate commits seems better, but that does lead to an
> intermediate state where the backlight sysfs interface will be broken
> (and fixed 2 commits later). See below for some background info.
>
> The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c
> are quite small and should not lead to any conflicts, so I believe that
> it would be best to merge this entire series through the drm-intel tree.
>
> Lee, may I have your Acked-by for merging the mfd change through the
> drm-intel tree?
>
> Rafael, may I have your Acked-by for merging the acpi_lpss change through the
> drm-intel tree?
>
> Regards,
>
> Hans
>
> p.s.
>
> The promised background info:
>
> We have this long standing issue where instead of looking in the i915
> VBT (Video BIOS Table) to see if we should use the PWM block of the SoC
> or of the PMIC to control the backlight of a DSI panel, we rely on
> drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c
> registering a pwm with the generic name of "pwm_backlight" and then the
> i915 panel code does a pwm_get(dev, "pwm_backlight").
>
> We have some heuristics in drivers/acpi/acpi_lpss.c to not register the
> lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c
> code simply assumes that since there is a PMIC the PMIC PWM block will
> be used. Basically we are winging it.
>
> Recently I've learned about 2 different BYT devices:
> Point of View MOBII TAB-P800W
> Acer Switch 10 SW5-012
>
> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
> PWM controller (and the VBT correctly indicates this), so here our old
> heuristics fail.
>
> This series renams the PWM lookups registered by the LPSS /
> intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp.
> "pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when
> to register the lookup. This combined with teaching the i915 panel to call
> pwm_get for the right lookup-name depending on the VBT bits resolves this.

Hans, thanks for your continued efforts in digging into the bottom of
this!

I'm sure there are a number of related bugs still open at fdo bugzilla.

It all makes sense,

Acked-by: Jani Nikula <[email protected]>

for merging through whichever tree.


Thanks,
Jani.


--
Jani Nikula, Intel Open Source Graphics Center

2019-11-19 15:50:02

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [PATCH 3/3] drm/i915: DSI: select correct PWM controller to use based on the VBT

On Tue, Nov 19, 2019 at 04:18:18PM +0100, Hans de Goede wrote:
> At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
> different PWM controllers for controlling the LCD's backlight brightness.
> Either the one integrated into the PMIC or the one integrated into the
> SoC (the 1st LPSS PWM controller).
>
> So far in the LPSS code on BYT we have skipped registering the LPSS PWM
> controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
> present, assuming that in this case the PMIC PWM controller will be used.
>
> On CHT we have been relying on only 1 of the 2 PWM controllers being
> enabled in the DSDT at the same time; and always registered the lookup.
>
> So far this has been working, but the correct way to determine which PWM
> controller needs to be used is by checking a bit in the VBT table and
> recently I've learned about 2 different BYT devices:
> Point of View MOBII TAB-P800W
> Acer Switch 10 SW5-012
>
> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
> PWM controller (and the VBT correctly indicates this), so here our old
> heuristics fail.
>
> This commit fixes using the wrong PWM controller on these devices by
> calling pwm_get() for the right PWM controller based on the
> VBT dsi.config.pwm_blc bit.
>
> Note this is part of a series which contains 2 other patches which renames
> the PWM lookup for the 1st SoC/LPSS PWM from "pwm_backlight" to
> "pwm_pmic_backlight" and the PWM lookup for the Crystal Cove PMIC PWM
> from "pwm_backlight" to "pwm_pmic_backlight".
>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_panel.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index bc14e9c0285a..ddcf311d1114 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -1840,13 +1840,22 @@ static int pwm_setup_backlight(struct intel_connector *connector,
> enum pipe pipe)
> {
> struct drm_device *dev = connector->base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_panel *panel = &connector->panel;
> + const char *desc;
> int retval;
>
> - /* Get the PWM chip for backlight control */
> - panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight");
> + /* Get the right PWM chip for DSI backlight according to VBT */
> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight");
> + desc = "PMIC";
> + } else {
> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight");
> + desc = "SoC";
> + }

Might we want the same thing for the panel enable gpio?

> +
> if (IS_ERR(panel->backlight.pwm)) {
> - DRM_ERROR("Failed to own the pwm chip\n");
> + DRM_ERROR("Failed to get the %s PWM chip\n", desc);
> panel->backlight.pwm = NULL;
> return -ENODEV;
> }
> @@ -1873,6 +1882,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,
> CRC_PMIC_PWM_PERIOD_NS);
> panel->backlight.enabled = panel->backlight.level != 0;
>
> + DRM_INFO("Using %s PWM for LCD backlight control\n", desc);
> return 0;
> }
>
> --
> 2.23.0

--
Ville Syrj?l?
Intel

2019-11-19 16:34:08

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/3] drm/i915 / LPSS / mfd: Select correct PWM controller to use based on VBT

On Tue, Nov 19, 2019 at 04:18:15PM +0100, Hans de Goede wrote:
> Hi All,
>
> This series needs to be merged through a single tree, to keep things
> bisectable. I have even considered just squashing all 3 patches into 1,
> but having separate commits seems better, but that does lead to an
> intermediate state where the backlight sysfs interface will be broken
> (and fixed 2 commits later). See below for some background info.
>
> The changes to drivers/acpi/acpi_lpss.c and drivers/mfd/intel_soc_pmic_core.c
> are quite small and should not lead to any conflicts, so I believe that
> it would be best to merge this entire series through the drm-intel tree.
>
> Lee, may I have your Acked-by for merging the mfd change through the
> drm-intel tree?
>
> Rafael, may I have your Acked-by for merging the acpi_lpss change through the
> drm-intel tree?
>

Entire series (or a single patch) makes sense to me.
Thanks for fixing this old hardware!

FWIW,
Reviewed-by: Andy Shevchenko <[email protected]>

> Regards,
>
> Hans
>
> p.s.
>
> The promised background info:
>
> We have this long standing issue where instead of looking in the i915
> VBT (Video BIOS Table) to see if we should use the PWM block of the SoC
> or of the PMIC to control the backlight of a DSI panel, we rely on
> drivers/acpi/acpi_lpss.c and/or drivers/mfd/intel_soc_pmic_core.c
> registering a pwm with the generic name of "pwm_backlight" and then the
> i915 panel code does a pwm_get(dev, "pwm_backlight").
>
> We have some heuristics in drivers/acpi/acpi_lpss.c to not register the
> lookup if a Crystal Cove PMIC is presend and the mfd/intel_soc_pmic_core.c
> code simply assumes that since there is a PMIC the PMIC PWM block will
> be used. Basically we are winging it.
>
> Recently I've learned about 2 different BYT devices:
> Point of View MOBII TAB-P800W
> Acer Switch 10 SW5-012
>
> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
> PWM controller (and the VBT correctly indicates this), so here our old
> heuristics fail.
>
> This series renams the PWM lookups registered by the LPSS /
> intel_soc_pmic_core.c code from "pwm_backlight" to "pwm_soc_backlight" resp.
> "pwm_pmic_backlight" and in the LPSS case also dropping the heuristics when
> to register the lookup. This combined with teaching the i915 panel to call
> pwm_get for the right lookup-name depending on the VBT bits resolves this.
>

--
With Best Regards,
Andy Shevchenko



2019-11-19 16:50:22

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 3/3] drm/i915: DSI: select correct PWM controller to use based on the VBT

Hi,

On 19-11-2019 16:47, Ville Syrj?l? wrote:
> On Tue, Nov 19, 2019 at 04:18:18PM +0100, Hans de Goede wrote:
>> At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
>> different PWM controllers for controlling the LCD's backlight brightness.
>> Either the one integrated into the PMIC or the one integrated into the
>> SoC (the 1st LPSS PWM controller).
>>
>> So far in the LPSS code on BYT we have skipped registering the LPSS PWM
>> controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
>> present, assuming that in this case the PMIC PWM controller will be used.
>>
>> On CHT we have been relying on only 1 of the 2 PWM controllers being
>> enabled in the DSDT at the same time; and always registered the lookup.
>>
>> So far this has been working, but the correct way to determine which PWM
>> controller needs to be used is by checking a bit in the VBT table and
>> recently I've learned about 2 different BYT devices:
>> Point of View MOBII TAB-P800W
>> Acer Switch 10 SW5-012
>>
>> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
>> PWM controller (and the VBT correctly indicates this), so here our old
>> heuristics fail.
>>
>> This commit fixes using the wrong PWM controller on these devices by
>> calling pwm_get() for the right PWM controller based on the
>> VBT dsi.config.pwm_blc bit.
>>
>> Note this is part of a series which contains 2 other patches which renames
>> the PWM lookup for the 1st SoC/LPSS PWM from "pwm_backlight" to
>> "pwm_pmic_backlight" and the PWM lookup for the Crystal Cove PMIC PWM
>> from "pwm_backlight" to "pwm_pmic_backlight".
>>
>> Signed-off-by: Hans de Goede <[email protected]>
>> ---
>> drivers/gpu/drm/i915/display/intel_panel.c | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
>> index bc14e9c0285a..ddcf311d1114 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
>> @@ -1840,13 +1840,22 @@ static int pwm_setup_backlight(struct intel_connector *connector,
>> enum pipe pipe)
>> {
>> struct drm_device *dev = connector->base.dev;
>> + struct drm_i915_private *dev_priv = to_i915(dev);
>> struct intel_panel *panel = &connector->panel;
>> + const char *desc;
>> int retval;
>>
>> - /* Get the PWM chip for backlight control */
>> - panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight");
>> + /* Get the right PWM chip for DSI backlight according to VBT */
>> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
>> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight");
>> + desc = "PMIC";
>> + } else {
>> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight");
>> + desc = "SoC";
>> + }
>
> Might we want the same thing for the panel enable gpio?


TL;DR: yes but that is for a separate series, which currently only exists in my head.

Longer story:

It looks like on BYT we need to control both VLV_GPIO_NC_10_PANEL1_BKLTEN and
VLV_GPIO_NC_11_PANEL1_BKLTCTL from vlv_dsi.c when the LPSS is used for PWM.
With BKLTCTL working as a panel_enable (needs to be driven high early on
when initializing the panel) and BKLTEN is just a backlight enable/disable
GPIO.

Without this DSI panels will not light-up on BYT when a HDMI monitor is
connected and the GOP chooses to initialize the HDMI rather then the panel,
since then these 2 pins stay low.

On CHT the MIPI power on/off sequences seem to take care of this themselves.

I still want to run some more tests. Currently if I export the 2 gpios in
question in sysfs (since their not claimed yet) and read them they always
read 0. I have the feeling this is caused by the input-buffer not being
enabled on these GPIOs, and that they really are high. So I want to do
a little hack to enable the input buffer and then see if indeed they
are high when the GOP has initialized the panel.

Testing has already shown that driving them high manualy before loading
i915 when the GOP did not init the panel fixes the panel not lighting up.
So I'm pretty sure that this is the case, but I want to verify this before
writing a series for that.

Regards,

Hans



>
>> +
>> if (IS_ERR(panel->backlight.pwm)) {
>> - DRM_ERROR("Failed to own the pwm chip\n");
>> + DRM_ERROR("Failed to get the %s PWM chip\n", desc);
>> panel->backlight.pwm = NULL;
>> return -ENODEV;
>> }
>> @@ -1873,6 +1882,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,
>> CRC_PMIC_PWM_PERIOD_NS);
>> panel->backlight.enabled = panel->backlight.level != 0;
>>
>> + DRM_INFO("Using %s PWM for LCD backlight control\n", desc);
>> return 0;
>> }
>>
>> --
>> 2.23.0
>


2019-12-10 08:53:20

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

On Tue, 19 Nov 2019, Hans de Goede wrote:

> At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
> different PWM controllers for controlling the LCD's backlight brightness.
>
> Either the one integrated into the PMIC or the one integrated into the
> SoC (the 1st LPSS PWM controller).
>
> So far in the LPSS code on BYT we have skipped registering the LPSS PWM
> controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
> present, assuming that in this case the PMIC PWM controller will be used.
>
> On CHT we have been relying on only 1 of the 2 PWM controllers being
> enabled in the DSDT at the same time; and always registered the lookup.
>
> So far this has been working, but the correct way to determine which PWM
> controller needs to be used is by checking a bit in the VBT table and
> recently I've learned about 2 different BYT devices:
> Point of View MOBII TAB-P800W
> Acer Switch 10 SW5-012
>
> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
> PWM controller (and the VBT correctly indicates this), so here our old
> heuristics fail.
>
> Since only the i915 driver has access to the VBT, this commit renames
> the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
> controller to "pwm_pmic_backlight" so that the i915 driver can do a
> pwm_get() for the right controller depending on the VBT bit, instead of
> the i915 driver relying on a "pwm_backlight" lookup getting registered
> which magically points to the right controller.
>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/mfd/intel_soc_pmic_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-12-11 17:30:29

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

Hi Lee,

On 10-12-2019 09:51, Lee Jones wrote:
> On Tue, 19 Nov 2019, Hans de Goede wrote:
>
>> At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
>> different PWM controllers for controlling the LCD's backlight brightness.
>>
>> Either the one integrated into the PMIC or the one integrated into the
>> SoC (the 1st LPSS PWM controller).
>>
>> So far in the LPSS code on BYT we have skipped registering the LPSS PWM
>> controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
>> present, assuming that in this case the PMIC PWM controller will be used.
>>
>> On CHT we have been relying on only 1 of the 2 PWM controllers being
>> enabled in the DSDT at the same time; and always registered the lookup.
>>
>> So far this has been working, but the correct way to determine which PWM
>> controller needs to be used is by checking a bit in the VBT table and
>> recently I've learned about 2 different BYT devices:
>> Point of View MOBII TAB-P800W
>> Acer Switch 10 SW5-012
>>
>> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
>> PWM controller (and the VBT correctly indicates this), so here our old
>> heuristics fail.
>>
>> Since only the i915 driver has access to the VBT, this commit renames
>> the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
>> controller to "pwm_pmic_backlight" so that the i915 driver can do a
>> pwm_get() for the right controller depending on the VBT bit, instead of
>> the i915 driver relying on a "pwm_backlight" lookup getting registered
>> which magically points to the right controller.
>>
>> Signed-off-by: Hans de Goede <[email protected]>
>> ---
>> drivers/mfd/intel_soc_pmic_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> For my own reference:
> Acked-for-MFD-by: Lee Jones <[email protected]>

As mentioned in the cover-letter, to avoid breaking bi-sectability
as well as to avoid breaking the intel-gfx CI we need to merge this series
in one go through one tree. Specifically through the drm-intel tree.
Is that ok with you ?

If this is ok with you, then you do not have to do anything, I will just push
the entire series to drm-intel. drivers/mfd/intel_soc_pmic_core.c
does not see much changes so I do not expect this to lead to any conflicts.

Regards,

Hans

2019-12-12 08:46:43

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

On Wed, 11 Dec 2019, Hans de Goede wrote:

> Hi Lee,
>
> On 10-12-2019 09:51, Lee Jones wrote:
> > On Tue, 19 Nov 2019, Hans de Goede wrote:
> >
> > > At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
> > > different PWM controllers for controlling the LCD's backlight brightness.
> > >
> > > Either the one integrated into the PMIC or the one integrated into the
> > > SoC (the 1st LPSS PWM controller).
> > >
> > > So far in the LPSS code on BYT we have skipped registering the LPSS PWM
> > > controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
> > > present, assuming that in this case the PMIC PWM controller will be used.
> > >
> > > On CHT we have been relying on only 1 of the 2 PWM controllers being
> > > enabled in the DSDT at the same time; and always registered the lookup.
> > >
> > > So far this has been working, but the correct way to determine which PWM
> > > controller needs to be used is by checking a bit in the VBT table and
> > > recently I've learned about 2 different BYT devices:
> > > Point of View MOBII TAB-P800W
> > > Acer Switch 10 SW5-012
> > >
> > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
> > > PWM controller (and the VBT correctly indicates this), so here our old
> > > heuristics fail.
> > >
> > > Since only the i915 driver has access to the VBT, this commit renames
> > > the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
> > > controller to "pwm_pmic_backlight" so that the i915 driver can do a
> > > pwm_get() for the right controller depending on the VBT bit, instead of
> > > the i915 driver relying on a "pwm_backlight" lookup getting registered
> > > which magically points to the right controller.
> > >
> > > Signed-off-by: Hans de Goede <[email protected]>
> > > ---
> > > drivers/mfd/intel_soc_pmic_core.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > For my own reference:
> > Acked-for-MFD-by: Lee Jones <[email protected]>
>
> As mentioned in the cover-letter, to avoid breaking bi-sectability
> as well as to avoid breaking the intel-gfx CI we need to merge this series
> in one go through one tree. Specifically through the drm-intel tree.
> Is that ok with you ?
>
> If this is ok with you, then you do not have to do anything, I will just push
> the entire series to drm-intel. drivers/mfd/intel_soc_pmic_core.c
> does not see much changes so I do not expect this to lead to any conflicts.

It's fine, so long as a minimal immutable pull-request is provided.
Whether it's pulled or not will depend on a number of factors, but it
needs to be an option.

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-12-12 14:35:43

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

Hi,

On 12-12-2019 09:45, Lee Jones wrote:
> On Wed, 11 Dec 2019, Hans de Goede wrote:
>
>> Hi Lee,
>>
>> On 10-12-2019 09:51, Lee Jones wrote:
>>> On Tue, 19 Nov 2019, Hans de Goede wrote:
>>>
>>>> At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
>>>> different PWM controllers for controlling the LCD's backlight brightness.
>>>>
>>>> Either the one integrated into the PMIC or the one integrated into the
>>>> SoC (the 1st LPSS PWM controller).
>>>>
>>>> So far in the LPSS code on BYT we have skipped registering the LPSS PWM
>>>> controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
>>>> present, assuming that in this case the PMIC PWM controller will be used.
>>>>
>>>> On CHT we have been relying on only 1 of the 2 PWM controllers being
>>>> enabled in the DSDT at the same time; and always registered the lookup.
>>>>
>>>> So far this has been working, but the correct way to determine which PWM
>>>> controller needs to be used is by checking a bit in the VBT table and
>>>> recently I've learned about 2 different BYT devices:
>>>> Point of View MOBII TAB-P800W
>>>> Acer Switch 10 SW5-012
>>>>
>>>> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
>>>> PWM controller (and the VBT correctly indicates this), so here our old
>>>> heuristics fail.
>>>>
>>>> Since only the i915 driver has access to the VBT, this commit renames
>>>> the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
>>>> controller to "pwm_pmic_backlight" so that the i915 driver can do a
>>>> pwm_get() for the right controller depending on the VBT bit, instead of
>>>> the i915 driver relying on a "pwm_backlight" lookup getting registered
>>>> which magically points to the right controller.
>>>>
>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>> ---
>>>> drivers/mfd/intel_soc_pmic_core.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> For my own reference:
>>> Acked-for-MFD-by: Lee Jones <[email protected]>
>>
>> As mentioned in the cover-letter, to avoid breaking bi-sectability
>> as well as to avoid breaking the intel-gfx CI we need to merge this series
>> in one go through one tree. Specifically through the drm-intel tree.
>> Is that ok with you ?
>>
>> If this is ok with you, then you do not have to do anything, I will just push
>> the entire series to drm-intel. drivers/mfd/intel_soc_pmic_core.c
>> does not see much changes so I do not expect this to lead to any conflicts.
>
> It's fine, so long as a minimal immutable pull-request is provided.
> Whether it's pulled or not will depend on a number of factors, but it
> needs to be an option.

The way the drm subsys works that is not really a readily available
option. The struct definition which this patch changes a single line in
has not been touched since 2015-06-26 so I really doubt we will get a
conflict from this.

Regards,

Hans

2019-12-12 15:54:04

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

On Thu, 12 Dec 2019, Hans de Goede wrote:

> Hi,
>
> On 12-12-2019 09:45, Lee Jones wrote:
> > On Wed, 11 Dec 2019, Hans de Goede wrote:
> >
> > > Hi Lee,
> > >
> > > On 10-12-2019 09:51, Lee Jones wrote:
> > > > On Tue, 19 Nov 2019, Hans de Goede wrote:
> > > >
> > > > > At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
> > > > > different PWM controllers for controlling the LCD's backlight brightness.
> > > > >
> > > > > Either the one integrated into the PMIC or the one integrated into the
> > > > > SoC (the 1st LPSS PWM controller).
> > > > >
> > > > > So far in the LPSS code on BYT we have skipped registering the LPSS PWM
> > > > > controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
> > > > > present, assuming that in this case the PMIC PWM controller will be used.
> > > > >
> > > > > On CHT we have been relying on only 1 of the 2 PWM controllers being
> > > > > enabled in the DSDT at the same time; and always registered the lookup.
> > > > >
> > > > > So far this has been working, but the correct way to determine which PWM
> > > > > controller needs to be used is by checking a bit in the VBT table and
> > > > > recently I've learned about 2 different BYT devices:
> > > > > Point of View MOBII TAB-P800W
> > > > > Acer Switch 10 SW5-012
> > > > >
> > > > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
> > > > > PWM controller (and the VBT correctly indicates this), so here our old
> > > > > heuristics fail.
> > > > >
> > > > > Since only the i915 driver has access to the VBT, this commit renames
> > > > > the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
> > > > > controller to "pwm_pmic_backlight" so that the i915 driver can do a
> > > > > pwm_get() for the right controller depending on the VBT bit, instead of
> > > > > the i915 driver relying on a "pwm_backlight" lookup getting registered
> > > > > which magically points to the right controller.
> > > > >
> > > > > Signed-off-by: Hans de Goede <[email protected]>
> > > > > ---
> > > > > drivers/mfd/intel_soc_pmic_core.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > For my own reference:
> > > > Acked-for-MFD-by: Lee Jones <[email protected]>
> > >
> > > As mentioned in the cover-letter, to avoid breaking bi-sectability
> > > as well as to avoid breaking the intel-gfx CI we need to merge this series
> > > in one go through one tree. Specifically through the drm-intel tree.
> > > Is that ok with you ?
> > >
> > > If this is ok with you, then you do not have to do anything, I will just push
> > > the entire series to drm-intel. drivers/mfd/intel_soc_pmic_core.c
> > > does not see much changes so I do not expect this to lead to any conflicts.
> >
> > It's fine, so long as a minimal immutable pull-request is provided.
> > Whether it's pulled or not will depend on a number of factors, but it
> > needs to be an option.
>
> The way the drm subsys works that is not really a readily available
> option. The struct definition which this patch changes a single line in
> has not been touched since 2015-06-26 so I really doubt we will get a
> conflict from this.

Always with the exceptions ...

OOI, why does this *have* to go through the DRM tree?

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-12-12 19:04:58

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

Hi,

On 12-12-2019 16:52, Lee Jones wrote:
> On Thu, 12 Dec 2019, Hans de Goede wrote:
>
>> Hi,
>>
>> On 12-12-2019 09:45, Lee Jones wrote:
>>> On Wed, 11 Dec 2019, Hans de Goede wrote:
>>>
>>>> Hi Lee,
>>>>
>>>> On 10-12-2019 09:51, Lee Jones wrote:
>>>>> On Tue, 19 Nov 2019, Hans de Goede wrote:
>>>>>
>>>>>> At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
>>>>>> different PWM controllers for controlling the LCD's backlight brightness.
>>>>>>
>>>>>> Either the one integrated into the PMIC or the one integrated into the
>>>>>> SoC (the 1st LPSS PWM controller).
>>>>>>
>>>>>> So far in the LPSS code on BYT we have skipped registering the LPSS PWM
>>>>>> controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
>>>>>> present, assuming that in this case the PMIC PWM controller will be used.
>>>>>>
>>>>>> On CHT we have been relying on only 1 of the 2 PWM controllers being
>>>>>> enabled in the DSDT at the same time; and always registered the lookup.
>>>>>>
>>>>>> So far this has been working, but the correct way to determine which PWM
>>>>>> controller needs to be used is by checking a bit in the VBT table and
>>>>>> recently I've learned about 2 different BYT devices:
>>>>>> Point of View MOBII TAB-P800W
>>>>>> Acer Switch 10 SW5-012
>>>>>>
>>>>>> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
>>>>>> PWM controller (and the VBT correctly indicates this), so here our old
>>>>>> heuristics fail.
>>>>>>
>>>>>> Since only the i915 driver has access to the VBT, this commit renames
>>>>>> the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
>>>>>> controller to "pwm_pmic_backlight" so that the i915 driver can do a
>>>>>> pwm_get() for the right controller depending on the VBT bit, instead of
>>>>>> the i915 driver relying on a "pwm_backlight" lookup getting registered
>>>>>> which magically points to the right controller.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>>>> ---
>>>>>> drivers/mfd/intel_soc_pmic_core.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> For my own reference:
>>>>> Acked-for-MFD-by: Lee Jones <[email protected]>
>>>>
>>>> As mentioned in the cover-letter, to avoid breaking bi-sectability
>>>> as well as to avoid breaking the intel-gfx CI we need to merge this series
>>>> in one go through one tree. Specifically through the drm-intel tree.
>>>> Is that ok with you ?
>>>>
>>>> If this is ok with you, then you do not have to do anything, I will just push
>>>> the entire series to drm-intel. drivers/mfd/intel_soc_pmic_core.c
>>>> does not see much changes so I do not expect this to lead to any conflicts.
>>>
>>> It's fine, so long as a minimal immutable pull-request is provided.
>>> Whether it's pulled or not will depend on a number of factors, but it
>>> needs to be an option.
>>
>> The way the drm subsys works that is not really a readily available
>> option. The struct definition which this patch changes a single line in
>> has not been touched since 2015-06-26 so I really doubt we will get a
>> conflict from this.
>
> Always with the exceptions ...
>
> OOI, why does this *have* to go through the DRM tree?

This patch renames the name used to lookup the pwm controller from
"pwm_backlight" to "pwm_pmic_backlight" because there are 2 possible
pwm controllers which may be used, one in the SoC itself and one
in the PMIC. Which controller should be used is described in a table
in the Video BIOS, so another part of this series adds this code to
the i915 driver:

- panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight");
+ /* Get the right PWM chip for DSI backlight according to VBT */
+ if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
+ panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight");
+ desc = "PMIC";
+ } else {
+ panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight");
+ desc = "SoC";
+ }

So both not to break bisectability, but also so as to not break the extensive
CI system which is used to test the i915 driver we need the MFD change doing
the rename to go upstrream through the same tree as the i915 change.

I have even considered just squashing the 2 commits together as having only 1
present, but not the other breaks stuff left and right.

Regards,

Hans

2019-12-13 08:28:44

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

On Thu, 12 Dec 2019, Hans de Goede wrote:

> Hi,
>
> On 12-12-2019 16:52, Lee Jones wrote:
> > On Thu, 12 Dec 2019, Hans de Goede wrote:
> >
> > > Hi,
> > >
> > > On 12-12-2019 09:45, Lee Jones wrote:
> > > > On Wed, 11 Dec 2019, Hans de Goede wrote:
> > > >
> > > > > Hi Lee,
> > > > >
> > > > > On 10-12-2019 09:51, Lee Jones wrote:
> > > > > > On Tue, 19 Nov 2019, Hans de Goede wrote:
> > > > > >
> > > > > > > At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
> > > > > > > different PWM controllers for controlling the LCD's backlight brightness.
> > > > > > >
> > > > > > > Either the one integrated into the PMIC or the one integrated into the
> > > > > > > SoC (the 1st LPSS PWM controller).
> > > > > > >
> > > > > > > So far in the LPSS code on BYT we have skipped registering the LPSS PWM
> > > > > > > controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
> > > > > > > present, assuming that in this case the PMIC PWM controller will be used.
> > > > > > >
> > > > > > > On CHT we have been relying on only 1 of the 2 PWM controllers being
> > > > > > > enabled in the DSDT at the same time; and always registered the lookup.
> > > > > > >
> > > > > > > So far this has been working, but the correct way to determine which PWM
> > > > > > > controller needs to be used is by checking a bit in the VBT table and
> > > > > > > recently I've learned about 2 different BYT devices:
> > > > > > > Point of View MOBII TAB-P800W
> > > > > > > Acer Switch 10 SW5-012
> > > > > > >
> > > > > > > Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
> > > > > > > PWM controller (and the VBT correctly indicates this), so here our old
> > > > > > > heuristics fail.
> > > > > > >
> > > > > > > Since only the i915 driver has access to the VBT, this commit renames
> > > > > > > the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
> > > > > > > controller to "pwm_pmic_backlight" so that the i915 driver can do a
> > > > > > > pwm_get() for the right controller depending on the VBT bit, instead of
> > > > > > > the i915 driver relying on a "pwm_backlight" lookup getting registered
> > > > > > > which magically points to the right controller.
> > > > > > >
> > > > > > > Signed-off-by: Hans de Goede <[email protected]>
> > > > > > > ---
> > > > > > > drivers/mfd/intel_soc_pmic_core.c | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > For my own reference:
> > > > > > Acked-for-MFD-by: Lee Jones <[email protected]>
> > > > >
> > > > > As mentioned in the cover-letter, to avoid breaking bi-sectability
> > > > > as well as to avoid breaking the intel-gfx CI we need to merge this series
> > > > > in one go through one tree. Specifically through the drm-intel tree.
> > > > > Is that ok with you ?
> > > > >
> > > > > If this is ok with you, then you do not have to do anything, I will just push
> > > > > the entire series to drm-intel. drivers/mfd/intel_soc_pmic_core.c
> > > > > does not see much changes so I do not expect this to lead to any conflicts.
> > > >
> > > > It's fine, so long as a minimal immutable pull-request is provided.
> > > > Whether it's pulled or not will depend on a number of factors, but it
> > > > needs to be an option.
> > >
> > > The way the drm subsys works that is not really a readily available
> > > option. The struct definition which this patch changes a single line in
> > > has not been touched since 2015-06-26 so I really doubt we will get a
> > > conflict from this.
> >
> > Always with the exceptions ...
> >
> > OOI, why does this *have* to go through the DRM tree?
>
> This patch renames the name used to lookup the pwm controller from
> "pwm_backlight" to "pwm_pmic_backlight" because there are 2 possible
> pwm controllers which may be used, one in the SoC itself and one
> in the PMIC. Which controller should be used is described in a table
> in the Video BIOS, so another part of this series adds this code to
> the i915 driver:
>
> - panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight");
> + /* Get the right PWM chip for DSI backlight according to VBT */
> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight");
> + desc = "PMIC";
> + } else {
> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight");
> + desc = "SoC";
> + }
>
> So both not to break bisectability, but also so as to not break the extensive
> CI system which is used to test the i915 driver we need the MFD change doing
> the rename to go upstrream through the same tree as the i915 change.
>
> I have even considered just squashing the 2 commits together as having only 1
> present, but not the other breaks stuff left and right.

That doesn't answer the question.

Why do they all *have* to go in via the DRM tree specifically?

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2019-12-13 12:41:58

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 2/3] mfd: intel_soc_pmic: Rename pwm_backlight pwm-lookup to pwm_pmic_backlight

Hi,

On 13-12-2019 09:27, Lee Jones wrote:
> On Thu, 12 Dec 2019, Hans de Goede wrote:
>
>> Hi,
>>
>> On 12-12-2019 16:52, Lee Jones wrote:
>>> On Thu, 12 Dec 2019, Hans de Goede wrote:
>>>
>>>> Hi,
>>>>
>>>> On 12-12-2019 09:45, Lee Jones wrote:
>>>>> On Wed, 11 Dec 2019, Hans de Goede wrote:
>>>>>
>>>>>> Hi Lee,
>>>>>>
>>>>>> On 10-12-2019 09:51, Lee Jones wrote:
>>>>>>> On Tue, 19 Nov 2019, Hans de Goede wrote:
>>>>>>>
>>>>>>>> At least Bay Trail (BYT) and Cherry Trail (CHT) devices can use 1 of 2
>>>>>>>> different PWM controllers for controlling the LCD's backlight brightness.
>>>>>>>>
>>>>>>>> Either the one integrated into the PMIC or the one integrated into the
>>>>>>>> SoC (the 1st LPSS PWM controller).
>>>>>>>>
>>>>>>>> So far in the LPSS code on BYT we have skipped registering the LPSS PWM
>>>>>>>> controller "pwm_backlight" lookup entry when a Crystal Cove PMIC is
>>>>>>>> present, assuming that in this case the PMIC PWM controller will be used.
>>>>>>>>
>>>>>>>> On CHT we have been relying on only 1 of the 2 PWM controllers being
>>>>>>>> enabled in the DSDT at the same time; and always registered the lookup.
>>>>>>>>
>>>>>>>> So far this has been working, but the correct way to determine which PWM
>>>>>>>> controller needs to be used is by checking a bit in the VBT table and
>>>>>>>> recently I've learned about 2 different BYT devices:
>>>>>>>> Point of View MOBII TAB-P800W
>>>>>>>> Acer Switch 10 SW5-012
>>>>>>>>
>>>>>>>> Which use a Crystal Cove PMIC, yet the LCD is connected to the SoC/LPSS
>>>>>>>> PWM controller (and the VBT correctly indicates this), so here our old
>>>>>>>> heuristics fail.
>>>>>>>>
>>>>>>>> Since only the i915 driver has access to the VBT, this commit renames
>>>>>>>> the "pwm_backlight" lookup entries for the Crystal Cove PMIC's PWM
>>>>>>>> controller to "pwm_pmic_backlight" so that the i915 driver can do a
>>>>>>>> pwm_get() for the right controller depending on the VBT bit, instead of
>>>>>>>> the i915 driver relying on a "pwm_backlight" lookup getting registered
>>>>>>>> which magically points to the right controller.
>>>>>>>>
>>>>>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>>>>>> ---
>>>>>>>> drivers/mfd/intel_soc_pmic_core.c | 2 +-
>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> For my own reference:
>>>>>>> Acked-for-MFD-by: Lee Jones <[email protected]>
>>>>>>
>>>>>> As mentioned in the cover-letter, to avoid breaking bi-sectability
>>>>>> as well as to avoid breaking the intel-gfx CI we need to merge this series
>>>>>> in one go through one tree. Specifically through the drm-intel tree.
>>>>>> Is that ok with you ?
>>>>>>
>>>>>> If this is ok with you, then you do not have to do anything, I will just push
>>>>>> the entire series to drm-intel. drivers/mfd/intel_soc_pmic_core.c
>>>>>> does not see much changes so I do not expect this to lead to any conflicts.
>>>>>
>>>>> It's fine, so long as a minimal immutable pull-request is provided.
>>>>> Whether it's pulled or not will depend on a number of factors, but it
>>>>> needs to be an option.
>>>>
>>>> The way the drm subsys works that is not really a readily available
>>>> option. The struct definition which this patch changes a single line in
>>>> has not been touched since 2015-06-26 so I really doubt we will get a
>>>> conflict from this.
>>>
>>> Always with the exceptions ...
>>>
>>> OOI, why does this *have* to go through the DRM tree?
>>
>> This patch renames the name used to lookup the pwm controller from
>> "pwm_backlight" to "pwm_pmic_backlight" because there are 2 possible
>> pwm controllers which may be used, one in the SoC itself and one
>> in the PMIC. Which controller should be used is described in a table
>> in the Video BIOS, so another part of this series adds this code to
>> the i915 driver:
>>
>> - panel->backlight.pwm = pwm_get(dev->dev, "pwm_backlight");
>> + /* Get the right PWM chip for DSI backlight according to VBT */
>> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
>> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_pmic_backlight");
>> + desc = "PMIC";
>> + } else {
>> + panel->backlight.pwm = pwm_get(dev->dev, "pwm_soc_backlight");
>> + desc = "SoC";
>> + }
>>
>> So both not to break bisectability, but also so as to not break the extensive
>> CI system which is used to test the i915 driver we need the MFD change doing
>> the rename to go upstrream through the same tree as the i915 change.
>>
>> I have even considered just squashing the 2 commits together as having only 1
>> present, but not the other breaks stuff left and right.
>
> That doesn't answer the question.
>
> Why do they all *have* to go in via the DRM tree specifically?

1. As explained these chanegs need to stay together
2. This change is primarily a drm/i915 change. Also the i915 code sees lots
of changes every cycle, where as the change to the mfd code touches a block
of code which has not been touched since 2015-06-26, so the chance of conflicts
is much bigger if this goes on through another tree.

I honestly do not see the problem here? Let me reverse the question why should this
NOT go in through the drm tree?

Regards,

Hans