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

2019-12-18 Thread Lee Jones
On Wed, 18 Dec 2019, Jani Nikula wrote:

> On Tue, 17 Dec 2019, Lee Jones  wrote:
> > Hans was making the case that this was impractical for DRM, due to the
> > amount of churn you guys receive, hence the discussion.  I'm very
> > pleased that this is not the case.
> 
> Heh, well, it is the case, but the point is that should be our problem,
> not yours. ;)

:)

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-17 Thread Jani Nikula
On Tue, 17 Dec 2019, Lee Jones  wrote:
> Hans was making the case that this was impractical for DRM, due to the
> amount of churn you guys receive, hence the discussion.  I'm very
> pleased that this is not the case.

Heh, well, it is the case, but the point is that should be our problem,
not yours. ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-17 Thread Lee Jones
On Tue, 17 Dec 2019, Jani Nikula wrote:

> On Tue, 17 Dec 2019, Lee Jones  wrote:
> > On Mon, 16 Dec 2019, Hans de Goede wrote:
> >
> >> Hi,
> >> 
> >> Doing immutable branches assumes that there is a base point,
> >> e.g. 5.5-rc1 where the immutable branch can then be based on and
> >> that the branch can then be merged without issues into both subsystems.
> >> 
> >> drm is constantly evolving to deal with and mostly catch up with new
> >> hardware as both GPUs and display-pipelines are evolving quite rapidly
> >> atm drm-intel-next has about 400 commits on top of 5.5-rc1 so for an
> >> immutable branch I can either base it on drm-intel-next which
> >> violates your request for a clean minimal branch to merge; or I can
> >> base it on 5.5-rc1 which leads to a big chance of problems when
> >> merging it given to large amount of churn in drm-intel-next.
> >
> > This is a *slightly* more compelling reason than the ones you've
> > previously provided.
> >
> >> So instead of the normal case of 2 subsystems seeing some changes
> >> on both side the case we have here is a part of a file which has
> >> not changed since 2015-06-26 in one subsys (and changing only
> >> a single line there!) and OTOH we have bigger changes to a subsys
> >> which see 400 patches land in the first week since rc1 .
> >
> > This is not.
> >
> >> I hope that you agree that in this case given the large amount of
> >> churn in drm-intel-next it makes since to just straight forward
> >> apply these patches on top of drm-intel-next.
> >
> > I have Acked this patch, but remember *this* is the exception rather
> > than the rule.  If/when we have a case where a contributor works
> > cross-subsystem with DRM and the code/file adapted is live (more
> > likely to change), I will have to insist on an immutable branch
> > strategy.  DRM will have to deal with that appropriately.
> 
> Hi, thanks for the ack and reaching an agreement with Hans, and sorry
> for not responding earlier.
> 
> It's not unusual for us to have topic branches for cross-subsystem or
> cross-driver changes, and I think usually we try to be accommodating in
> merging stuff through whichever tree it makes most sense. In fact my ack
> to do just that was my first response on this series [1].
> 
> So I don't really know why the fuss. We'll anyway deal with any
> cross-subsystem series on a case by case basis, depending on what makes
> most sense, and what suits all maintainers involved.

Perfect.  Thanks for the clarification.  I look forward to working
with you guys in the future.

Hans was making the case that this was impractical for DRM, due to the
amount of churn you guys receive, hence the discussion.  I'm very
pleased that this is not the case.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-17 Thread Jani Nikula
On Tue, 17 Dec 2019, Lee Jones  wrote:
> On Mon, 16 Dec 2019, Hans de Goede wrote:
>
>> Hi,
>> 
>> Doing immutable branches assumes that there is a base point,
>> e.g. 5.5-rc1 where the immutable branch can then be based on and
>> that the branch can then be merged without issues into both subsystems.
>> 
>> drm is constantly evolving to deal with and mostly catch up with new
>> hardware as both GPUs and display-pipelines are evolving quite rapidly
>> atm drm-intel-next has about 400 commits on top of 5.5-rc1 so for an
>> immutable branch I can either base it on drm-intel-next which
>> violates your request for a clean minimal branch to merge; or I can
>> base it on 5.5-rc1 which leads to a big chance of problems when
>> merging it given to large amount of churn in drm-intel-next.
>
> This is a *slightly* more compelling reason than the ones you've
> previously provided.
>
>> So instead of the normal case of 2 subsystems seeing some changes
>> on both side the case we have here is a part of a file which has
>> not changed since 2015-06-26 in one subsys (and changing only
>> a single line there!) and OTOH we have bigger changes to a subsys
>> which see 400 patches land in the first week since rc1 .
>
> This is not.
>
>> I hope that you agree that in this case given the large amount of
>> churn in drm-intel-next it makes since to just straight forward
>> apply these patches on top of drm-intel-next.
>
> I have Acked this patch, but remember *this* is the exception rather
> than the rule.  If/when we have a case where a contributor works
> cross-subsystem with DRM and the code/file adapted is live (more
> likely to change), I will have to insist on an immutable branch
> strategy.  DRM will have to deal with that appropriately.

Hi, thanks for the ack and reaching an agreement with Hans, and sorry
for not responding earlier.

It's not unusual for us to have topic branches for cross-subsystem or
cross-driver changes, and I think usually we try to be accommodating in
merging stuff through whichever tree it makes most sense. In fact my ack
to do just that was my first response on this series [1].

So I don't really know why the fuss. We'll anyway deal with any
cross-subsystem series on a case by case basis, depending on what makes
most sense, and what suits all maintainers involved.


Thanks again,
Jani.


[1] http://mid.mail-archive.com/87pnhnyir8.fsf@intel.com



-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-17 Thread Lee Jones
On Mon, 16 Dec 2019, Hans de Goede wrote:

> Hi,
> 
> On 16-12-2019 10:30, Lee Jones wrote:
> > [...]
> > 
> > > > > > > > > > > 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 
> > > > > > > > > > > ---
> > > > > > > > > > >   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 
> > > > > > > > > 
> > > > > > > > > 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?
> > 

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

2019-12-16 Thread Hans de Goede

Hi,

On 16-12-2019 10:30, Lee Jones wrote:

[...]


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 
---
  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 


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?


There isn't a problem with *this* patch.  I could say, "sure, take it"
and the chances are everything could be fine from a technical
perspective.

However, I'm taking exception to the fact you think this series is
*special* enough to warrant circumventing the usual way in which we
usually work when dealing with cross-subsystem patch-sets.  Something
I personally deal with a lot due to the inherent hierarchical nature
of Multi-Functional Devices.

I'm on the fence on this one.  Due to the circumstances surrounding
*this* patch alone, it would be so much easier (for both of us!) to
just Ack the patch and hope no further changes occur which could
potentially cause someone else (you, me, Linus) more work later on.
However, I'm very keen to prevent setting a precedent for this kind of
action, as it's clearly not the right path to take in a vast majority
of cases.


1. As explained these chanegs need to stay together


The patch-set would stay together regardless.  That's the point of an
immutable branch, it can be taken in by all relevant parties and Git
will just do-the-right-thing.


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.


This too is irrelevant, since the patch-set could/would go though
both/all trees simultaneously.  The way in which we normally work with
other 

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

2019-12-16 Thread Lee Jones
[...]

> > > > > > > > > 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 
> > > > > > > > > ---
> > > > > > > > >  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 
> > > > > > > 
> > > > > > > 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?

There isn't a problem with *this* patch.  I could say, "sure, take it"
and the chances are everything could be fine from a technical
perspective.

However, I'm taking exception to the fact you think this series is
*special* enough to warrant circumventing the usual way in which we
usually work when dealing with cross-subsystem patch-sets.  Something
I personally deal with a lot due to the inherent hierarchical nature
of Multi-Functional Devices.

I'm on the fence on this one.  Due to the circumstances surrounding
*this* patch alone, it would be so much easier (for both of us!) to
just Ack the patch and hope no further changes occur 

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

2019-12-13 Thread Hans de Goede

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 
---
 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 


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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-13 Thread Lee Jones
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 
> > > > > > > ---
> > > > > > > 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 
> > > > > 
> > > > > 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 

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

2019-12-12 Thread Hans de Goede

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 
---
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 


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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-12 Thread Lee Jones
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 
> > > > > ---
> > > > >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 
> > > 
> > > 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-12 Thread Hans de Goede

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 
---
   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 


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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-12 Thread Lee Jones
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 
> > > ---
> > >   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 
> 
> 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-11 Thread Hans de Goede

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 
---
  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 


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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-12-10 Thread Lee Jones
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 
> ---
>  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 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2019-11-19 Thread Hans de Goede
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 
---
 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, ":00:02.0", "pwm_backlight", 0, 
PWM_POLARITY_NORMAL),
+   PWM_LOOKUP("crystal_cove_pwm", 0, ":00:02.0", "pwm_pmic_backlight", 
0, PWM_POLARITY_NORMAL),
 };
 
 static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c,
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel