| Message ID | 1350595304-18237-5-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 18, 2012 at 06:21:34PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > We need to check if any of the pipes is using TRANSCODER_EDP. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> I wonder whether it doesn't make more sense for haswell to return the transcoder in the encoder->get_hw_state function, and then map that to the crtc with the intel_pipe_to_cpu_transcoder. That way we don't need to add a special-case for eDP. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 73ce007..827c5ba 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8551,6 +8551,31 @@ void intel_modeset_setup_hw_state(struct drm_device *dev) > struct intel_encoder *encoder; > struct intel_connector *connector; > > + if (IS_HASWELL(dev)) { > + tmp = I915_READ(DDI_FUNC_CTL(TRANSCODER_EDP)); > + > + if (tmp & TRANS_DDI_FUNC_ENABLE) { > + switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { > + case TRANS_DDI_EDP_INPUT_A_ON: > + case TRANS_DDI_EDP_INPUT_A_ONOFF: > + pipe = PIPE_A; > + break; > + case TRANS_DDI_EDP_INPUT_B_ONOFF: > + pipe = PIPE_B; > + break; > + case TRANS_DDI_EDP_INPUT_C_ONOFF: > + pipe = PIPE_C; > + break; > + } > + > + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > + crtc->cpu_transcoder = TRANSCODER_EDP; > + > + DRM_DEBUG_KMS("Pipe %c using transcoder EDP\n", > + pipe_name(pipe)); > + } > + } > + > for_each_pipe(pipe) { > crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > -- > 1.7.11.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi 2012/10/18 Daniel Vetter <daniel@ffwll.ch>: > On Thu, Oct 18, 2012 at 06:21:34PM -0300, Paulo Zanoni wrote: >> From: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> We need to check if any of the pipes is using TRANSCODER_EDP. >> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > I wonder whether it doesn't make more sense for haswell to return the > transcoder in the encoder->get_hw_state function, and then map that to the > crtc with the intel_pipe_to_cpu_transcoder. That way we don't need to add > a special-case for eDP. I know we had some IRC discussions about this comment, but after this I tried to implement your suggestion. It won't work unless we completely rewrite a lot of code. For example, on intel_modeset_setup_hw_state (the function this patch changes) we need to know which transcoder is associated to the pipe before doing I915_READ(PIPECONF(pipe)) (because PIPECONF(pipe) is wrong, we need PIPECONF(cpu_transcoder), which comes later in the patch series), so we will need to discover the encoder state before discovering the pipe state, which is not what we do now. Also, having dev_priv->transcoder_to_pipe_mapping is not as clean as having pipe_to_transcoder_mapping because every pipe is always associated with a transcoder, but not every transcoder is associated to a pipe, so we'd need to keep checking for NULL on unused transcoders. My first strategy to implement this code also tried to use "for_each_transcoder" instead of "for_each_pipe" when checking the pipe state, but it added even more complexity to the function. I really believe my approach here is as KISS as we can get. A single special haswell case here prevents a lot of "is this cpu transcoder being used by some pipe?" checks in a lot of places. > -Daniel > >> --- >> drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 73ce007..827c5ba 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -8551,6 +8551,31 @@ void intel_modeset_setup_hw_state(struct drm_device *dev) >> struct intel_encoder *encoder; >> struct intel_connector *connector; >> >> + if (IS_HASWELL(dev)) { >> + tmp = I915_READ(DDI_FUNC_CTL(TRANSCODER_EDP)); >> + >> + if (tmp & TRANS_DDI_FUNC_ENABLE) { >> + switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { >> + case TRANS_DDI_EDP_INPUT_A_ON: >> + case TRANS_DDI_EDP_INPUT_A_ONOFF: >> + pipe = PIPE_A; >> + break; >> + case TRANS_DDI_EDP_INPUT_B_ONOFF: >> + pipe = PIPE_B; >> + break; >> + case TRANS_DDI_EDP_INPUT_C_ONOFF: >> + pipe = PIPE_C; >> + break; >> + } >> + >> + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); >> + crtc->cpu_transcoder = TRANSCODER_EDP; >> + >> + DRM_DEBUG_KMS("Pipe %c using transcoder EDP\n", >> + pipe_name(pipe)); >> + } >> + } >> + >> for_each_pipe(pipe) { >> crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); >> >> -- >> 1.7.11.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 73ce007..827c5ba 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8551,6 +8551,31 @@ void intel_modeset_setup_hw_state(struct drm_device *dev) struct intel_encoder *encoder; struct intel_connector *connector; + if (IS_HASWELL(dev)) { + tmp = I915_READ(DDI_FUNC_CTL(TRANSCODER_EDP)); + + if (tmp & TRANS_DDI_FUNC_ENABLE) { + switch (tmp & TRANS_DDI_EDP_INPUT_MASK) { + case TRANS_DDI_EDP_INPUT_A_ON: + case TRANS_DDI_EDP_INPUT_A_ONOFF: + pipe = PIPE_A; + break; + case TRANS_DDI_EDP_INPUT_B_ONOFF: + pipe = PIPE_B; + break; + case TRANS_DDI_EDP_INPUT_C_ONOFF: + pipe = PIPE_C; + break; + } + + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); + crtc->cpu_transcoder = TRANSCODER_EDP; + + DRM_DEBUG_KMS("Pipe %c using transcoder EDP\n", + pipe_name(pipe)); + } + } + for_each_pipe(pipe) { crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);