On Wed, Jul 26, 2017 at 4:33 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
