On Thu, Aug 16, 2012 at 4:23 PM, Wang, Xingchao <xingchao.w...@intel.com> wrote: > > >> -----Original Message----- >> From: Imre Deak [mailto:imre.d...@gmail.com] >> Sent: Thursday, August 16, 2012 6:54 PM >> To: Wang, Xingchao >> Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org; przan...@gmail.com >> Subject: Re: [PATCH v7 3/4] drm/i915: Haswell HDMI audio initialization >> >> On Thu, Aug 16, 2012 at 6:45 AM, Wang Xingchao <xingchao.w...@intel.com> >> wrote: >> > On Wed, Aug 15, 2012 at 08:05:14PM +0300, Imre Deak wrote: >> >> On Wed, Aug 15, 2012 at 6:27 AM, Wang, Xingchao >> <xingchao.w...@intel.com> wrote: >> >> [...] >> >> >> + I915_WRITE(aud_cntrl_st2, tmp); >> >> >> + >> >> >> + /* Wait for 1 vertical blank */ >> >> >> + intel_wait_for_vblank(dev, pipe); >> >> >> + >> >> >> + /* Set ELD valid state */ >> >> >> + tmp = I915_READ(aud_cntrl_st2); >> >> >> + DRM_DEBUG_DRIVER("HDMI audio: pin eld vld status=0x%8x\n", >> tmp); >> >> >> + tmp |= (AUDIO_ELD_VALID_A << (pipe * 4)); >> >> >> + I915_WRITE(aud_cntrl_st2, tmp); >> >> >> + tmp = I915_READ(aud_cntrl_st2); >> >> >> + DRM_DEBUG_DRIVER("HDMI audio: eld vld status=0x%8x\n", >> tmp); >> >> >> + >> >> >> + /* Enable HDMI mode */ >> >> >> + tmp = I915_READ(aud_config); >> >> >> + DRM_DEBUG_DRIVER("HDMI audio: audio conf: 0x%8x\n", tmp); >> >> >> + /* clear N_programing_enable and N_value_index */ >> >> >> + tmp &= ~(AUD_CONFIG_N_VALUE_INDEX | >> >> >> AUD_CONFIG_N_PROG_ENABLE); >> >> >> + I915_WRITE(aud_config, tmp); >> >> >> + >> >> >> + DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(pipe)); >> >> >> + >> >> >> + i = I915_READ(aud_cntl_st); >> >> >> + i = (i >> 29) & DIP_PORT_SEL_MASK; /* >> DIP_Port_Select, 0x1 = >> >> >> PortB */ >> >> >> + if (!i) { >> >> >> + DRM_DEBUG_DRIVER("Audio directed to unknown >> port\n"); >> >> >> + /* operate blindly on all ports */ >> >> >> + eldv = AUDIO_ELD_VALID_A; >> >> >> + eldv |= AUDIO_ELD_VALID_B; >> >> >> + eldv |= AUDIO_ELD_VALID_C; >> >> >> + } else { >> >> >> + DRM_DEBUG_DRIVER("ELD on port %c\n", 'A' + i); >> >> >> + eldv = AUDIO_ELD_VALID_A << ((i - 1) * 4); >> >> >> + } >> >> >> >> Again, if we handle the ELD_VALID bits on a transcoder basis, as >> >> above when enabling and disabling it, why are we doing it here >> >> differently, on a port basis? >> > >> > A bit different. These bits[30:29] reflects which port is used to >> > transmit DIP data. It's configured in other register, see >> > PIPE_DDI_FUNC_CTL, that determines which DDI port would be combined >> > with current pipe. I think it's also transcoder basis. please note >> > aud_cntl_st is >> also "pipe" based. >> >> On new HW it shouldn't matter which port is used to transmit the DIP data for >> the ELD configuration. Earlier in the code you have picked the ELD_VALID_X >> bit >> based on the pipe: >> >> tmp |= (AUDIO_ELD_VALID_A << (pipe * 4)); >> >> and written this to the AUD_PIN_ELD_CP_VLD register . Here you pick the >> same bit based on the port: >> >> eldv = AUDIO_ELD_VALID_A << ((i - 1) * 4); >> >> and write this to the same AUD_PIN_ELD_CP_VLD register. The definition from >> the spec for this register is the same though in both cases: the ELD valid >> bit >> should be picked based on the transcoder, no matter what port is used to >> transfer the data. > > Thanks for clarification. As it's pipe/transcoder basis for ELD bits on > Haswell, I agree just set the specific active transcoder, > For older hardware, we just left the code there, is that okay for you?
Yes, I presume then that all older HW versions (including DevHSW:GT0:X0) are handled in another stub function. >> I cannot test this right now, since I don't have an HSW machine set up here. >> Could you Wang give a try to the following diff on top of your patch? > > I tested your patch and it just works well as before. It's more clear now. Thanks for your explanations, it's also clearer for me now :) > I will add your change to patch and send it to Daniel, is that okay? Yes, it's ok, you can also add my r-b then. --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
