| Message ID | 1363946021-26651-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/3/22 Daniel Vetter <daniel.vetter@ffwll.ch>: > This reverts commit cc464b2a17c59adedbdc02cc54341d630354edc3. > > The reason is that Takashi Iwai reported a regression bisected to this > commit: > > http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg18788.html > > His machine has eDP on port D (usual desktop all-in-on setup), which > intel_dp.c identifies as an eDP panel, but the hsw ddi code > mishandles. > > But in retrospective I also don't like that this leaks highly platform > specific details into common code, and the reason is that the drm > vblank layer sucks. So instead I think we should: > - move the cpu_transcoder into the dynamic pipe_config tracking (once > that's merged). > - fix up the drm vblank layer to finally deal with kms crtc objects > instead of int pipes. Looks like we'll never get rid of all the unclaimed register errors... Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Takashi Iwai <tiwai@suse.de> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/intel_display.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7114e68..8eec205 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5742,6 +5742,11 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, > num_connectors++; > } > > + if (is_cpu_edp) > + intel_crtc->cpu_transcoder = TRANSCODER_EDP; > + else > + intel_crtc->cpu_transcoder = pipe; > + > /* We are not sure yet this won't happen. */ > WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n", > INTEL_PCH_TYPE(dev)); > @@ -5808,11 +5813,6 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, > int pipe = intel_crtc->pipe; > int ret; > > - if (IS_HASWELL(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) > - intel_crtc->cpu_transcoder = TRANSCODER_EDP; > - else > - intel_crtc->cpu_transcoder = pipe; > - > drm_vblank_pre_modeset(dev, pipe); > > ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode, > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Mar 22, 2013 at 10:53 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > This reverts commit cc464b2a17c59adedbdc02cc54341d630354edc3. > > The reason is that Takashi Iwai reported a regression bisected to this > commit: > > http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg18788.html > > His machine has eDP on port D (usual desktop all-in-on setup), which > intel_dp.c identifies as an eDP panel, but the hsw ddi code > mishandles. Closer inspection of the code reveals that haswell_crtc_mode_set also checks intel_encoder_is_pch_edp when setting is_cpu_edp. On haswell that doesn't make much sense (since there's no edp on the pch), but what this function _really_ checks is whether that edp connector is on port A or port D. It's just that on ilk-ivb port D was on the pch ... So that explains why this seemingly innocent change killed eDP on port D. Furthermore it looks like everything else accidentally works, since we've never enabled eDP on port D support for hsw intentionally (e.g. we still register the HDMI output for port D in that case). I'll add that to the commit message when merging. -Daniel
On Fri, Mar 22, 2013 at 09:30:05AM -0300, Paulo Zanoni wrote: > 2013/3/22 Daniel Vetter <daniel.vetter@ffwll.ch>: > > This reverts commit cc464b2a17c59adedbdc02cc54341d630354edc3. > > > > The reason is that Takashi Iwai reported a regression bisected to this > > commit: > > > > http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg18788.html > > > > His machine has eDP on port D (usual desktop all-in-on setup), which > > intel_dp.c identifies as an eDP panel, but the hsw ddi code > > mishandles. > > > > But in retrospective I also don't like that this leaks highly platform > > specific details into common code, and the reason is that the drm > > vblank layer sucks. So instead I think we should: > > - move the cpu_transcoder into the dynamic pipe_config tracking (once > > that's merged). > > - fix up the drm vblank layer to finally deal with kms crtc objects > > instead of int pipes. > > Looks like we'll never get rid of all the unclaimed register errors... > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Picked up for -fixes, thanks for the review. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7114e68..8eec205 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5742,6 +5742,11 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, num_connectors++; } + if (is_cpu_edp) + intel_crtc->cpu_transcoder = TRANSCODER_EDP; + else + intel_crtc->cpu_transcoder = pipe; + /* We are not sure yet this won't happen. */ WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev)); @@ -5808,11 +5813,6 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, int pipe = intel_crtc->pipe; int ret; - if (IS_HASWELL(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) - intel_crtc->cpu_transcoder = TRANSCODER_EDP; - else - intel_crtc->cpu_transcoder = pipe; - drm_vblank_pre_modeset(dev, pipe); ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
This reverts commit cc464b2a17c59adedbdc02cc54341d630354edc3. The reason is that Takashi Iwai reported a regression bisected to this commit: http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg18788.html His machine has eDP on port D (usual desktop all-in-on setup), which intel_dp.c identifies as an eDP panel, but the hsw ddi code mishandles. But in retrospective I also don't like that this leaks highly platform specific details into common code, and the reason is that the drm vblank layer sucks. So instead I think we should: - move the cpu_transcoder into the dynamic pipe_config tracking (once that's merged). - fix up the drm vblank layer to finally deal with kms crtc objects instead of int pipes. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Takashi Iwai <tiwai@suse.de> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)