Re: [PATCH v2] drm/i915/bxt: add bxt dsi gpio element support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 21 Dec 2016 08:45:46 +0200
Mika Kahola <mika.kahola@xxxxxxxxx> wrote:

> Hi Bob,
> 
> On Tue, 2016-12-20 at 09:53 -0800, Bob Paauwe wrote:
> > On Tue, 13 Dec 2016 16:11:20 +0200
> > Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
> >   
> > > 
> > > On Mon, 05 Dec 2016, Mika Kahola <mika.kahola@xxxxxxxxx> wrote:  
> > > > 
> > > > From: Jani Nikula <jani.nikula@xxxxxxxxx>
> > > > 
> > > > Request the GPIO by index through the consumer API. For now, use
> > > > a quick
> > > > hack to store the already requested ones, simply because I have
> > > > no idea
> > > > whether this actually works or not, and I have no way to test it.
> > > > 
> > > > v2: switch *NULL* to *"panel"* when requesting gpio for MIPI/DSI
> > > > panel. (Mika)
> > > >   
> > Jani, Mika,
> > 
> > I'm working on getting a dual-link MIPI panel working on BXT and have
> > a
> > problem getting the proper GPIO pins set.  This patch gets things
> > closer, but at least for the platform I'm working on, there's one
> > GPIO
> > pin (backlight/panel control) that doesn't seem to be available via
> > the
> > consumer API.  If I use the alternate#2 method that Jani posted
> > (legacy GPIO API) along with the proper defines I can set this pin.
> > I'm
> > using:
> > 
> > 
> > #define BXT_PANEL1_VDDEN_PIN       196
> > #define BXT_PANEL1_BKLTEN_PIN      197
> > #define BXT_PANEL1_BKLTCTL_PIN     198
> > 
> > #define BXT_PANEL1_VDDEN_OFFSET    366
> > #define BXT_PANEL1_BKLTEN_OFFSET   367
> > #define BXT_PANEL1_BKLTCTL_OFFSET  368
> > 
> > to create my GPIO table.  
> > 
> > These three pins need to be set VDDEN, BKLTCTL, BKLTEN, in that order
> > to get the panel to turn on.
> > 
> > For testing, I have a call in the vbt_panel_prepare() to set the pin
> > and one in vbt_panel_unprepare() to clear it.  That works but that
> > doesn't seem like the right solution.  I was wondering if either of
> > you
> > have any insights?  
> Do you see these warnings in dmesg such as 'GPIO index x request
> failed'? This is what you should see if the pin can't be accessed. 

Yes, I get that message.  I dumped some additional information in the
gpio code and it shows that the pin count is 3 (and it's zero index
based).  Through the power on and backlight sequences, bxt_exec_gpio is
called with index 1 and 2. Both of those work and correspond to the
196 and 197 defines I listed above. If I call bxt_exec_gpio with index
0, it fails and I've also tried using index 3 and it fails.

Using 

devm_gpio_request_one(dev_priv->drm.dev, 368, GPIOF_DIR_OUT, "MIPI DSI");
gpio_set_value(368, 1);

does work and lets me set the third pin.

My guess is that the ACPI table is missing an entry for this third pin
and hence, why devm_gpiod_get_index() fails.  It also seems like
setting this pin should be included in one of the MIPI sequences. But I
don't understand all of this well enough to be sure.

> 
> Can you see  all of these GPIO pins in /sys/kernel/debug/gpio file?

Yes. I even have scripts to query and set the 3 pins.  Those work fine
(if the driver isn't running and has control over the pins).

> 
> And you also need to enable pincontrol for Broxton in kernel config
> 
> CONFIG_PINCTRL_BROXTON=y

Yes, that is enabled.

> 
> 
> 
> > Bob
> >   
> > >   
> > > > 
> > > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> > > > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx>    
> > > Pushed to dinq.
> > > 
> > > BR,
> > > Jani.
> > >   
> > > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 38
> > > > +++++++++++++++++++++++++-----
> > > >  1 file changed, 32 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> > > > b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> > > > index 0d8ff00..dda678b 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> > > > @@ -29,6 +29,7 @@
> > > >  #include <drm/drm_edid.h>
> > > >  #include <drm/i915_drm.h>
> > > >  #include <drm/drm_panel.h>
> > > > +#include <linux/gpio/consumer.h>
> > > >  #include <linux/slab.h>
> > > >  #include <video/mipi_display.h>
> > > >  #include <asm/intel-mid.h>
> > > > @@ -304,19 +305,44 @@ static void chv_exec_gpio(struct
> > > > drm_i915_private *dev_priv,
> > > >  	mutex_unlock(&dev_priv->sb_lock);
> > > >  }
> > > >  
> > > > +static void bxt_exec_gpio(struct drm_i915_private *dev_priv,
> > > > +			  u8 gpio_source, u8 gpio_index, bool
> > > > value)
> > > > +{
> > > > +	/* XXX: this table is a quick ugly hack. */
> > > > +	static struct gpio_desc *bxt_gpio_table[U8_MAX + 1];
> > > > +	struct gpio_desc *gpio_desc =
> > > > bxt_gpio_table[gpio_index];
> > > > +
> > > > +	if (!gpio_desc) {
> > > > +		gpio_desc = devm_gpiod_get_index(dev_priv-  
> > > > >drm.dev,  
> > > > +						 "panel",
> > > > gpio_index,
> > > > +						 value ?
> > > > GPIOD_OUT_LOW :
> > > > +						 GPIOD_OUT_HIGH)
> > > > ;
> > > > +
> > > > +		if (IS_ERR_OR_NULL(gpio_desc)) {
> > > > +			DRM_ERROR("GPIO index %u request failed
> > > > (%ld)\n",
> > > > +				  gpio_index,
> > > > PTR_ERR(gpio_desc));
> > > > +			return;
> > > > +		}
> > > > +
> > > > +		bxt_gpio_table[gpio_index] = gpio_desc;
> > > > +	}
> > > > +
> > > > +	gpiod_set_value(gpio_desc, value);
> > > > +}
> > > > +
> > > >  static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi,
> > > > const u8 *data)
> > > >  {
> > > >  	struct drm_device *dev = intel_dsi->base.base.dev;
> > > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > > > -	u8 gpio_source, gpio_index;
> > > > +	u8 gpio_source, gpio_index = 0, gpio_number;
> > > >  	bool value;
> > > >  
> > > >  	DRM_DEBUG_KMS("\n");
> > > >  
> > > >  	if (dev_priv->vbt.dsi.seq_version >= 3)
> > > > -		data++;
> > > > +		gpio_index = *data++;
> > > >  
> > > > -	gpio_index = *data++;
> > > > +	gpio_number = *data++;
> > > >  
> > > >  	/* gpio source in sequence v2 only */
> > > >  	if (dev_priv->vbt.dsi.seq_version == 2)
> > > > @@ -328,11 +354,11 @@ static const u8 *mipi_exec_gpio(struct
> > > > intel_dsi *intel_dsi, const u8 *data)
> > > >  	value = *data++ & 1;
> > > >  
> > > >  	if (IS_VALLEYVIEW(dev_priv))
> > > > -		vlv_exec_gpio(dev_priv, gpio_source, gpio_index,
> > > > value);
> > > > +		vlv_exec_gpio(dev_priv, gpio_source,
> > > > gpio_number, value);
> > > >  	else if (IS_CHERRYVIEW(dev_priv))
> > > > -		chv_exec_gpio(dev_priv, gpio_source, gpio_index,
> > > > value);
> > > > +		chv_exec_gpio(dev_priv, gpio_source,
> > > > gpio_number, value);
> > > >  	else
> > > > -		DRM_DEBUG_KMS("GPIO element not supported on
> > > > this platform\n");
> > > > +		bxt_exec_gpio(dev_priv, gpio_source, gpio_index,
> > > > value);
> > > >  
> > > >  	return data;
> > > >  }    
> > 
> >   



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@xxxxxxxxx
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux