| Message ID | 51e603fa921dec2b137d759a86c6169d26b461be.1350911444.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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>
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.
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
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;
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(-)