| Message ID | 20170725134841.2167-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Daniel Vetter (2017-07-25 14:48:41) > The hdmi bits simply don't exist, so nerf them. I think audio doesn't > work on gen3 at all, and for the limited color range we should > probably use the colorimetry sdvo paramater instead of the bit in the > port. > > But fixing sdvo isn't my goal, I just want to get the backtrace out of > the way, and this takes care of that. > > Still, while at it fix the missing read-out of the gen4 audio bit, > maybe that part even works ... > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/intel_sdvo.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c > index bea8152ae859..0403e30dfabc 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -1113,6 +1113,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config, > struct drm_connector_state *conn_state) > { > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_sdvo *intel_sdvo = to_sdvo(encoder); > struct intel_sdvo_connector_state *intel_sdvo_state = > to_intel_sdvo_connector_state(conn_state); > @@ -1174,6 +1175,12 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, > pipe_config->limited_color_range = true; > } > > + /* gen3 doesn't do the hdmi bits in the SDVO register */ > + if (INTEL_GEN(dev_priv) < 4) { > + pipe_config->limited_color_range = false; > + pipe_config->has_audio = false; > + } So historically, this would be if (!intel_sdvo->is_hdmi) { pipe_config->limited_color_range = false; pipe_config->has_audio = false; } Currently we set pipe_config->has_audio = intel_sdvo_state->base.force_audio == HDMI_AUDIO_ON || (intel_sdvo_state->base.force_audio == HDMI_AUDIO_AUTO && intel_sdvo->has_hdmi_audio)) and, with a slight paraphrase, pipe_config->limited_color_range = pipe_config->has_hdmi_sink; So, both of these should only be set if we have hdmi features which gen3 will not. So I think the consistent fix is switch (intel_sdvo_state->base.force_audio) { case HDMI_AUDIO_ON: pipe_config->has_audio = intel_sdvo->is_hdmi; break; case HDMI_AUDIO_AUTO: pipe_config->has_audio = intel_sdvo->has_hdmi_audio; break; case HDMI_AUDIO_OFF: case HDMI_AUDIO_OFF_DVI: pipe_config->has_audio = false; break; } Though I'm not sure what the actual subtlety of OFF_DVI is. > + > /* Clock computation needs to happen after pixel multiplier. */ > if (intel_sdvo->is_tv) > i9xx_adjust_sdvo_tv_clock(pipe_config); > @@ -1480,6 +1487,9 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, > if (sdvox & HDMI_COLOR_RANGE_16_235) > pipe_config->limited_color_range = true; > > + if (sdvox & SDVO_AUDIO_ENABLE) > + pipe_config->has_audio = true; This seems to be its own little fix that looks correct. -Chris
On Wed, Jul 26, 2017 at 4:33 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Daniel Vetter (2017-07-25 14:48:41) >> The hdmi bits simply don't exist, so nerf them. I think audio doesn't >> work on gen3 at all, and for the limited color range we should >> probably use the colorimetry sdvo paramater instead of the bit in the >> port. >> >> But fixing sdvo isn't my goal, I just want to get the backtrace out of >> the way, and this takes care of that. >> >> Still, while at it fix the missing read-out of the gen4 audio bit, >> maybe that part even works ... >> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> drivers/gpu/drm/i915/intel_sdvo.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c >> index bea8152ae859..0403e30dfabc 100644 >> --- a/drivers/gpu/drm/i915/intel_sdvo.c >> +++ b/drivers/gpu/drm/i915/intel_sdvo.c >> @@ -1113,6 +1113,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, >> struct intel_crtc_state *pipe_config, >> struct drm_connector_state *conn_state) >> { >> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> struct intel_sdvo *intel_sdvo = to_sdvo(encoder); >> struct intel_sdvo_connector_state *intel_sdvo_state = >> to_intel_sdvo_connector_state(conn_state); >> @@ -1174,6 +1175,12 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, >> pipe_config->limited_color_range = true; >> } >> >> + /* gen3 doesn't do the hdmi bits in the SDVO register */ >> + if (INTEL_GEN(dev_priv) < 4) { >> + pipe_config->limited_color_range = false; >> + pipe_config->has_audio = false; >> + } > > So historically, this would be > > if (!intel_sdvo->is_hdmi) { > pipe_config->limited_color_range = false; > pipe_config->has_audio = false; > } > > Currently we set > > pipe_config->has_audio = > intel_sdvo_state->base.force_audio == HDMI_AUDIO_ON || > (intel_sdvo_state->base.force_audio == HDMI_AUDIO_AUTO && intel_sdvo->has_hdmi_audio)) > > and, with a slight paraphrase, > > pipe_config->limited_color_range = pipe_config->has_hdmi_sink; > > So, both of these should only be set if we have hdmi features which gen3 > will not. So I think the consistent fix is > > switch (intel_sdvo_state->base.force_audio) { > case HDMI_AUDIO_ON: > pipe_config->has_audio = intel_sdvo->is_hdmi; > break; > case HDMI_AUDIO_AUTO: > pipe_config->has_audio = intel_sdvo->has_hdmi_audio; > break; > case HDMI_AUDIO_OFF: > case HDMI_AUDIO_OFF_DVI: > pipe_config->has_audio = false; > break; > } > > Though I'm not sure what the actual subtlety of OFF_DVI is. is_hdmi is set when the SDVO transcoder supports hdmi. And you can plug a hdmi sdvo chip into a gen3 box, which is why this blows up here. I guess I could try to limit that, that should take care of things complete. Together with making sure we don't set any of the hdmi state bits if the is_hdmi is false (which I hope we already do). -Daniel > >> + >> /* Clock computation needs to happen after pixel multiplier. */ >> if (intel_sdvo->is_tv) >> i9xx_adjust_sdvo_tv_clock(pipe_config); >> @@ -1480,6 +1487,9 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, >> if (sdvox & HDMI_COLOR_RANGE_16_235) >> pipe_config->limited_color_range = true; >> >> + if (sdvox & SDVO_AUDIO_ENABLE) >> + pipe_config->has_audio = true; > > This seems to be its own little fix that looks correct. > -Chris
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index bea8152ae859..0403e30dfabc 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1113,6 +1113,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) { + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_sdvo *intel_sdvo = to_sdvo(encoder); struct intel_sdvo_connector_state *intel_sdvo_state = to_intel_sdvo_connector_state(conn_state); @@ -1174,6 +1175,12 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder, pipe_config->limited_color_range = true; } + /* gen3 doesn't do the hdmi bits in the SDVO register */ + if (INTEL_GEN(dev_priv) < 4) { + pipe_config->limited_color_range = false; + pipe_config->has_audio = false; + } + /* Clock computation needs to happen after pixel multiplier. */ if (intel_sdvo->is_tv) i9xx_adjust_sdvo_tv_clock(pipe_config); @@ -1480,6 +1487,9 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, if (sdvox & HDMI_COLOR_RANGE_16_235) pipe_config->limited_color_range = true; + if (sdvox & SDVO_AUDIO_ENABLE) + pipe_config->has_audio = true; + if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE, &val, 1)) { if (val == SDVO_ENCODE_HDMI)
The hdmi bits simply don't exist, so nerf them. I think audio doesn't work on gen3 at all, and for the limited color range we should probably use the colorimetry sdvo paramater instead of the bit in the port. But fixing sdvo isn't my goal, I just want to get the backtrace out of the way, and this takes care of that. Still, while at it fix the missing read-out of the gen4 audio bit, maybe that part even works ... Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_sdvo.c | 10 ++++++++++ 1 file changed, 10 insertions(+)