diff mbox

[v2] drm/i915/bxt: use NULL for GPIO connection ID

Message ID 20170817105541.63914-1-andriy.shevchenko@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andy Shevchenko Aug. 17, 2017, 10:55 a.m. UTC
The commit 213e08ad60ba
	("drm/i915/bxt: add bxt dsi gpio element support")
enables GPIO support for Broxton based platforms.

While using that API we might get into troubles in the future, because
we can't rely on label name in the driver since vendor firmware might
provide any GPIO pin there, e.g. "reset", and even mark it in _DSD (in
which case the request will fail).

To avoid inconsistency and potential issues we have two options:
a) generate GPIO ACPI mapping table and supply it via
   acpi_dev_add_driver_gpios(), or
b) just pass NULL as connection ID.

The b) approach is much simpler and would work since the driver relies
on GPIO indices only. Moreover, the _CRS fallback mechanism, when
requesting GPIO, has been made stricter, and supplying non-NULL
connection ID when neither _DSD, nor GPIO ACPI mapping is present, is
making request fail.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101921
Fixes: f10e4bf6632b ("gpio: acpi: Even more tighten up ACPI GPIO lookups")
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2:
 - adjust commit message for proper time tenses
 - add Fixes: and Bugzilla: tags
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mika Kahola Aug. 17, 2017, 11:06 a.m. UTC | #1
Tested with GLK + MIPI/DSI panel (AU Optronics B101UAN01)

Tested-by: Mika Kahola <mika.kahola@intel.com>

On Thu, 2017-08-17 at 13:55 +0300, Andy Shevchenko wrote:
> The commit 213e08ad60ba
> 	("drm/i915/bxt: add bxt dsi gpio element support")
> enables GPIO support for Broxton based platforms.
> 
> While using that API we might get into troubles in the future,
> because
> we can't rely on label name in the driver since vendor firmware might
> provide any GPIO pin there, e.g. "reset", and even mark it in _DSD
> (in
> which case the request will fail).
> 
> To avoid inconsistency and potential issues we have two options:
> a) generate GPIO ACPI mapping table and supply it via
>    acpi_dev_add_driver_gpios(), or
> b) just pass NULL as connection ID.
> 
> The b) approach is much simpler and would work since the driver
> relies
> on GPIO indices only. Moreover, the _CRS fallback mechanism, when
> requesting GPIO, has been made stricter, and supplying non-NULL
> connection ID when neither _DSD, nor GPIO ACPI mapping is present, is
> making request fail.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101921
> Fixes: f10e4bf6632b ("gpio: acpi: Even more tighten up ACPI GPIO
> lookups")
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2:
>  - adjust commit message for proper time tenses
>  - add Fixes: and Bugzilla: tags
>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index 7158c7ce9c09..91c07b0c8db9 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -306,7 +306,7 @@ static void bxt_exec_gpio(struct drm_i915_private
> *dev_priv,
>  
>  	if (!gpio_desc) {
>  		gpio_desc = devm_gpiod_get_index(dev_priv->drm.dev,
> -						 "panel",
> gpio_index,
> +						 NULL, gpio_index,
>  						 value ?
> GPIOD_OUT_LOW :
>  						 GPIOD_OUT_HIGH);
>
Jani Nikula Aug. 18, 2017, 8:04 a.m. UTC | #2
On Thu, 17 Aug 2017, Mika Kahola <mika.kahola@intel.com> wrote:
> Tested with GLK + MIPI/DSI panel (AU Optronics B101UAN01)
>
> Tested-by: Mika Kahola <mika.kahola@intel.com>

Do we have a bxt system available for testing this? With that,

Acked-by: Jani Nikula <jani.nikula@intel.com>

BR,
Jani.


>
> On Thu, 2017-08-17 at 13:55 +0300, Andy Shevchenko wrote:
>> The commit 213e08ad60ba
>> 	("drm/i915/bxt: add bxt dsi gpio element support")
>> enables GPIO support for Broxton based platforms.
>> 
>> While using that API we might get into troubles in the future,
>> because
>> we can't rely on label name in the driver since vendor firmware might
>> provide any GPIO pin there, e.g. "reset", and even mark it in _DSD
>> (in
>> which case the request will fail).
>> 
>> To avoid inconsistency and potential issues we have two options:
>> a) generate GPIO ACPI mapping table and supply it via
>>    acpi_dev_add_driver_gpios(), or
>> b) just pass NULL as connection ID.
>> 
>> The b) approach is much simpler and would work since the driver
>> relies
>> on GPIO indices only. Moreover, the _CRS fallback mechanism, when
>> requesting GPIO, has been made stricter, and supplying non-NULL
>> connection ID when neither _DSD, nor GPIO ACPI mapping is present, is
>> making request fail.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101921
>> Fixes: f10e4bf6632b ("gpio: acpi: Even more tighten up ACPI GPIO
>> lookups")
>> Cc: Mika Kahola <mika.kahola@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>> v2:
>>  - adjust commit message for proper time tenses
>>  - add Fixes: and Bugzilla: tags
>>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> index 7158c7ce9c09..91c07b0c8db9 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> @@ -306,7 +306,7 @@ static void bxt_exec_gpio(struct drm_i915_private
>> *dev_priv,
>>  
>>  	if (!gpio_desc) {
>>  		gpio_desc = devm_gpiod_get_index(dev_priv->drm.dev,
>> -						 "panel",
>> gpio_index,
>> +						 NULL, gpio_index,
>>  						 value ?
>> GPIOD_OUT_LOW :
>>  						 GPIOD_OUT_HIGH);
>>
Mika Kahola Aug. 18, 2017, 11:46 a.m. UTC | #3
On Thu, 2017-08-17 at 14:06 +0300, Mika Kahola wrote:
> Tested with GLK + MIPI/DSI panel (AU Optronics B101UAN01)
Tested also with APL + MIPI/DSI setup.

> 
> Tested-by: Mika Kahola <mika.kahola@intel.com>
> 
> On Thu, 2017-08-17 at 13:55 +0300, Andy Shevchenko wrote:
> > 
> > The commit 213e08ad60ba
> > 	("drm/i915/bxt: add bxt dsi gpio element support")
> > enables GPIO support for Broxton based platforms.
> > 
> > While using that API we might get into troubles in the future,
> > because
> > we can't rely on label name in the driver since vendor firmware
> > might
> > provide any GPIO pin there, e.g. "reset", and even mark it in _DSD
> > (in
> > which case the request will fail).
> > 
> > To avoid inconsistency and potential issues we have two options:
> > a) generate GPIO ACPI mapping table and supply it via
> >    acpi_dev_add_driver_gpios(), or
> > b) just pass NULL as connection ID.
> > 
> > The b) approach is much simpler and would work since the driver
> > relies
> > on GPIO indices only. Moreover, the _CRS fallback mechanism, when
> > requesting GPIO, has been made stricter, and supplying non-NULL
> > connection ID when neither _DSD, nor GPIO ACPI mapping is present,
> > is
> > making request fail.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101921
> > Fixes: f10e4bf6632b ("gpio: acpi: Even more tighten up ACPI GPIO
> > lookups")
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > v2:
> >  - adjust commit message for proper time tenses
> >  - add Fixes: and Bugzilla: tags
> >  drivers/gpu/drm/i915/intel_dsi_vbt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> > b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> > index 7158c7ce9c09..91c07b0c8db9 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> > @@ -306,7 +306,7 @@ static void bxt_exec_gpio(struct
> > drm_i915_private
> > *dev_priv,
> >  
> >  	if (!gpio_desc) {
> >  		gpio_desc = devm_gpiod_get_index(dev_priv-
> > >drm.dev,
> > -						 "panel",
> > gpio_index,
> > +						 NULL, gpio_index,
> >  						 value ?
> > GPIOD_OUT_LOW :
> >  						 GPIOD_OUT_HIGH);
> >  
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jani Nikula Aug. 18, 2017, 1:21 p.m. UTC | #4
On Fri, 18 Aug 2017, Mika Kahola <mika.kahola@intel.com> wrote:
> On Thu, 2017-08-17 at 14:06 +0300, Mika Kahola wrote:
>> Tested with GLK + MIPI/DSI panel (AU Optronics B101UAN01)
> Tested also with APL + MIPI/DSI setup.

Pushed to drm-intel-next-queued, thanks for the patch and testing.

BR,
Jani.


>
>> 
>> Tested-by: Mika Kahola <mika.kahola@intel.com>
>> 
>> On Thu, 2017-08-17 at 13:55 +0300, Andy Shevchenko wrote:
>> > 
>> > The commit 213e08ad60ba
>> > 	("drm/i915/bxt: add bxt dsi gpio element support")
>> > enables GPIO support for Broxton based platforms.
>> > 
>> > While using that API we might get into troubles in the future,
>> > because
>> > we can't rely on label name in the driver since vendor firmware
>> > might
>> > provide any GPIO pin there, e.g. "reset", and even mark it in _DSD
>> > (in
>> > which case the request will fail).
>> > 
>> > To avoid inconsistency and potential issues we have two options:
>> > a) generate GPIO ACPI mapping table and supply it via
>> >    acpi_dev_add_driver_gpios(), or
>> > b) just pass NULL as connection ID.
>> > 
>> > The b) approach is much simpler and would work since the driver
>> > relies
>> > on GPIO indices only. Moreover, the _CRS fallback mechanism, when
>> > requesting GPIO, has been made stricter, and supplying non-NULL
>> > connection ID when neither _DSD, nor GPIO ACPI mapping is present,
>> > is
>> > making request fail.
>> > 
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101921
>> > Fixes: f10e4bf6632b ("gpio: acpi: Even more tighten up ACPI GPIO
>> > lookups")
>> > Cc: Mika Kahola <mika.kahola@intel.com>
>> > Cc: Jani Nikula <jani.nikula@intel.com>
>> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > ---
>> > v2:
>> >  - adjust commit message for proper time tenses
>> >  - add Fixes: and Bugzilla: tags
>> >  drivers/gpu/drm/i915/intel_dsi_vbt.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> > b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> > index 7158c7ce9c09..91c07b0c8db9 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> > +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> > @@ -306,7 +306,7 @@ static void bxt_exec_gpio(struct
>> > drm_i915_private
>> > *dev_priv,
>> >  
>> >  	if (!gpio_desc) {
>> >  		gpio_desc = devm_gpiod_get_index(dev_priv-
>> > >drm.dev,
>> > -						 "panel",
>> > gpio_index,
>> > +						 NULL, gpio_index,
>> >  						 value ?
>> > GPIOD_OUT_LOW :
>> >  						 GPIOD_OUT_HIGH);
>> >  
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index 7158c7ce9c09..91c07b0c8db9 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -306,7 +306,7 @@  static void bxt_exec_gpio(struct drm_i915_private *dev_priv,
 
 	if (!gpio_desc) {
 		gpio_desc = devm_gpiod_get_index(dev_priv->drm.dev,
-						 "panel", gpio_index,
+						 NULL, gpio_index,
 						 value ? GPIOD_OUT_LOW :
 						 GPIOD_OUT_HIGH);