Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/sdvo: force GPIO bit-banging also on default pin
@ 2012-10-22 13:12 Jani Nikula
  2012-10-22 13:12 ` [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2012-10-22 13:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

commit 63abf3edaf42d0b9f278df90fe41c7ed4796b6b1
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Dec 8 16:48:21 2010 +0000

    drm/i915/sdvo: Only use the SDVO pin if it is in the valid range

added a default fallback if BIOS provides an invalid pin mapping, but
failed to force GPIO bit-banging on it. Finish the job, and also clean up
the function a bit. With bit-banging, setting the gmbus speed has no
effect, so drop it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 0007a4d..eab1277 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2042,17 +2042,15 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
 	else
 		mapping = &dev_priv->sdvo_mappings[1];
 
-	pin = GMBUS_PORT_DPB;
-	if (mapping->initialized)
+	if (mapping->initialized && intel_gmbus_is_port_valid(mapping->i2c_pin))
 		pin = mapping->i2c_pin;
+	else
+		pin = GMBUS_PORT_DPB;
 
-	if (intel_gmbus_is_port_valid(pin)) {
-		sdvo->i2c = intel_gmbus_get_adapter(dev_priv, pin);
-		intel_gmbus_set_speed(sdvo->i2c, GMBUS_RATE_1MHZ);
-		intel_gmbus_force_bit(sdvo->i2c, true);
-	} else {
-		sdvo->i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPB);
-	}
+	sdvo->i2c = intel_gmbus_get_adapter(dev_priv, pin);
+
+	/* gmbus is not supported with sdvo yet */
+	intel_gmbus_force_bit(sdvo->i2c, true);
 }
 
 static bool
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures
  2012-10-22 13:12 [PATCH 1/2] drm/i915/sdvo: force GPIO bit-banging also on default pin Jani Nikula
@ 2012-10-22 13:12 ` Jani Nikula
  2012-10-23  9:40   ` Chris Wilson
  2012-10-23 10:01   ` Mika Kuoppala
  0 siblings, 2 replies; 6+ messages in thread
From: Jani Nikula @ 2012-10-22 13:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

SDVOB may be multiplexed with HDMIB. If it's not SDVOB, the same i2c
adapter may be used for HDMIB, with the adjusted config (i.e. with GPIO
bit-banging instead of gmbus). Restore i2c adapter config before error
return from intel_sdvo_init(), letting HDMIB enjoy the joys of gmbus.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index eab1277..6103cf6 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2053,6 +2053,13 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
 	intel_gmbus_force_bit(sdvo->i2c, true);
 }
 
+/* undo any changes intel_sdvo_select_i2c_bus() did to sdvo->i2c */
+static void
+intel_sdvo_unselect_i2c_bus(struct intel_sdvo *sdvo)
+{
+	intel_gmbus_force_bit(sdvo->i2c, false);
+}
+
 static bool
 intel_sdvo_is_hdmi_connector(struct intel_sdvo *intel_sdvo, int device)
 {
@@ -2628,10 +2635,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
 	intel_sdvo->is_sdvob = is_sdvob;
 	intel_sdvo->slave_addr = intel_sdvo_get_slave_addr(dev, intel_sdvo) >> 1;
 	intel_sdvo_select_i2c_bus(dev_priv, intel_sdvo, sdvo_reg);
-	if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev)) {
-		kfree(intel_sdvo);
-		return false;
-	}
+	if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev))
+		goto err_i2c_bus;
 
 	/* encoder type will be decided later */
 	intel_encoder = &intel_sdvo->base;
@@ -2716,6 +2721,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
 err:
 	drm_encoder_cleanup(&intel_encoder->base);
 	i2c_del_adapter(&intel_sdvo->ddc);
+err_i2c_bus:
+	intel_sdvo_unselect_i2c_bus(intel_sdvo);
 	kfree(intel_sdvo);
 
 	return false;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures
  2012-10-22 13:12 ` [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures Jani Nikula
@ 2012-10-23  9:40   ` Chris Wilson
  2012-10-26  6:21     ` Jani Nikula
  2012-10-23 10:01   ` Mika Kuoppala
  1 sibling, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2012-10-23  9:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

On Mon, 22 Oct 2012 16:12:18 +0300, Jani Nikula <jani.nikula@intel.com> wrote:
> SDVOB may be multiplexed with HDMIB. If it's not SDVOB, the same i2c
> adapter may be used for HDMIB, with the adjusted config (i.e. with GPIO
> bit-banging instead of gmbus). Restore i2c adapter config before error
> return from intel_sdvo_init(), letting HDMIB enjoy the joys of gmbus.

I would personally not make the assumption that set_speed has no effect.
Disabling GMBUS is a hack that we should eventually lift.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures
  2012-10-22 13:12 ` [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures Jani Nikula
  2012-10-23  9:40   ` Chris Wilson
@ 2012-10-23 10:01   ` Mika Kuoppala
  1 sibling, 0 replies; 6+ messages in thread
From: Mika Kuoppala @ 2012-10-23 10:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

On Mon, 22 Oct 2012 16:12:18 +0300, Jani Nikula <jani.nikula@intel.com> wrote:
> SDVOB may be multiplexed with HDMIB. If it's not SDVOB, the same i2c
> adapter may be used for HDMIB, with the adjusted config (i.e. with GPIO
> bit-banging instead of gmbus). Restore i2c adapter config before error
> return from intel_sdvo_init(), letting HDMIB enjoy the joys of gmbus.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c |   15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index eab1277..6103cf6 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2053,6 +2053,13 @@ intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
>  	intel_gmbus_force_bit(sdvo->i2c, true);
>  }
>  
> +/* undo any changes intel_sdvo_select_i2c_bus() did to sdvo->i2c */
> +static void
> +intel_sdvo_unselect_i2c_bus(struct intel_sdvo *sdvo)
> +{
> +	intel_gmbus_force_bit(sdvo->i2c, false);
> +}
> +
>  static bool
>  intel_sdvo_is_hdmi_connector(struct intel_sdvo *intel_sdvo, int device)
>  {
> @@ -2628,10 +2635,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
>  	intel_sdvo->is_sdvob = is_sdvob;
>  	intel_sdvo->slave_addr = intel_sdvo_get_slave_addr(dev, intel_sdvo) >> 1;
>  	intel_sdvo_select_i2c_bus(dev_priv, intel_sdvo, sdvo_reg);
> -	if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev)) {
> -		kfree(intel_sdvo);
> -		return false;
> -	}
> +	if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev))
> +		goto err_i2c_bus;
>  
>  	/* encoder type will be decided later */
>  	intel_encoder = &intel_sdvo->base;
> @@ -2716,6 +2721,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
>  err:
>  	drm_encoder_cleanup(&intel_encoder->base);
>  	i2c_del_adapter(&intel_sdvo->ddc);
> +err_i2c_bus:
> +	intel_sdvo_unselect_i2c_bus(intel_sdvo);
>  	kfree(intel_sdvo);
>  
>  	return false;
> -- 

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures
  2012-10-23  9:40   ` Chris Wilson
@ 2012-10-26  6:21     ` Jani Nikula
  2012-10-26  8:27       ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2012-10-26  6:21 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Tue, 23 Oct 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, 22 Oct 2012 16:12:18 +0300, Jani Nikula <jani.nikula@intel.com> wrote:
>> SDVOB may be multiplexed with HDMIB. If it's not SDVOB, the same i2c
>> adapter may be used for HDMIB, with the adjusted config (i.e. with GPIO
>> bit-banging instead of gmbus). Restore i2c adapter config before error
>> return from intel_sdvo_init(), letting HDMIB enjoy the joys of gmbus.
>
> I would personally not make the assumption that set_speed has no effect.
> Disabling GMBUS is a hack that we should eventually lift.

I guess this comment was more about the whole, not just patch 2/2, but
Daniel merged 1/2 already. IIUC you would have wanted to keep set_speed
there, but shall we just leave that to when we start using GMBUS on
SDVO? Patch 2/2 is pretty straightforward, but will also need some love
when enabling GMBUS.

Thoughts?

BR,
Jani.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures
  2012-10-26  6:21     ` Jani Nikula
@ 2012-10-26  8:27       ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-10-26  8:27 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Oct 26, 2012 at 09:21:05AM +0300, Jani Nikula wrote:
> On Tue, 23 Oct 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Mon, 22 Oct 2012 16:12:18 +0300, Jani Nikula <jani.nikula@intel.com> wrote:
> >> SDVOB may be multiplexed with HDMIB. If it's not SDVOB, the same i2c
> >> adapter may be used for HDMIB, with the adjusted config (i.e. with GPIO
> >> bit-banging instead of gmbus). Restore i2c adapter config before error
> >> return from intel_sdvo_init(), letting HDMIB enjoy the joys of gmbus.
> >
> > I would personally not make the assumption that set_speed has no effect.
> > Disabling GMBUS is a hack that we should eventually lift.
> 
> I guess this comment was more about the whole, not just patch 2/2, but
> Daniel merged 1/2 already. IIUC you would have wanted to keep set_speed
> there, but shall we just leave that to when we start using GMBUS on
> SDVO? Patch 2/2 is pretty straightforward, but will also need some love
> when enabling GMBUS.

Actually I wanted to merge both patches already. I've reordered things a
bit and added a comment to explain why we'd like to use gmbus for sdvo
(2MHz clock is simply faster), but that we have bugs with gmbus and so
must fall back to bit banging.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-10-26 14:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-22 13:12 [PATCH 1/2] drm/i915/sdvo: force GPIO bit-banging also on default pin Jani Nikula
2012-10-22 13:12 ` [PATCH 2/2] drm/i915/sdvo: restore i2c adapter config on intel_sdvo_init() failures Jani Nikula
2012-10-23  9:40   ` Chris Wilson
2012-10-26  6:21     ` Jani Nikula
2012-10-26  8:27       ` Daniel Vetter
2012-10-23 10:01   ` Mika Kuoppala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox