| Message ID | 1349449561-3599-9-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 5, 2012 at 4:05 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Just set the only bit we need, everything else is either ignored on > HDMI or should be set to zero. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Is the assumption that intel_enable_ddi() will only ever be used by the HDMI encore likely to hold true for ever? looks like a general function that could be used by others to me. Also intel_disable_ddi() read the register back, so it'd be unfair to only touch intel_enable_ddi() if you want to do that. I'd just stay on the safe side and retain the programming here.
On Wed, Oct 10, 2012 at 03:27:59PM +0100, Lespiau, Damien wrote: > On Fri, Oct 5, 2012 at 4:05 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Just set the only bit we need, everything else is either ignored on > > HDMI or should be set to zero. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Is the assumption that intel_enable_ddi() will only ever be used by > the HDMI encore likely to hold true for ever? looks like a general > function that could be used by others to me. Also intel_disable_ddi() > read the register back, so it'd be unfair to only touch > intel_enable_ddi() if you want to do that. > > I'd just stay on the safe side and retain the programming here. I'd prefer to go safe and do our own programming ;-) Imo it's better to fully program a register than to rely on random garbage left behind by the bios - usually that allows us to bring up hw a bit quicker, but in the end results in some nasty bug reports once hw starts shipping. For the disable side things are usually not that important, since we need to assume that our own code (or our own hw state readout) set things up correctly. Wrt the general usefullness of, I guess the dp patches will changed that. I'll just merge this one here. -Daniel
On Wed, Oct 10, 2012 at 04:56:07PM +0200, Daniel Vetter wrote: > On Wed, Oct 10, 2012 at 03:27:59PM +0100, Lespiau, Damien wrote: > > On Fri, Oct 5, 2012 at 4:05 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > > Just set the only bit we need, everything else is either ignored on > > > HDMI or should be set to zero. > > > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Is the assumption that intel_enable_ddi() will only ever be used by > > the HDMI encore likely to hold true for ever? looks like a general > > function that could be used by others to me. Also intel_disable_ddi() > > read the register back, so it'd be unfair to only touch > > intel_enable_ddi() if you want to do that. > > > > I'd just stay on the safe side and retain the programming here. > > I'd prefer to go safe and do our own programming ;-) Imo it's better to > fully program a register than to rely on random garbage left behind by the > bios - usually that allows us to bring up hw a bit quicker, but in the end > results in some nasty bug reports once hw starts shipping. For the disable > side things are usually not that important, since we need to assume that > our own code (or our own hw state readout) set things up correctly. > > Wrt the general usefullness of, I guess the dp patches will changed that. > I'll just merge this one here. Ok, I've merged this series up to this patch. The last two patches are pending an r-b from a volunteer ;-) Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index f713980..a4e05d0 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1017,16 +1017,12 @@ void intel_enable_ddi(struct intel_encoder *encoder) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); int port = intel_hdmi->ddi_port; - u32 temp; - - temp = I915_READ(DDI_BUF_CTL(port)); - temp |= DDI_BUF_CTL_ENABLE; /* Enable DDI_BUF_CTL. In HDMI/DVI mode, the port width, * and swing/emphasis values are ignored so nothing special needs * to be done besides enabling the port. */ - I915_WRITE(DDI_BUF_CTL(port), temp); + I915_WRITE(DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE); } void intel_disable_ddi(struct intel_encoder *encoder)