On Mon, 27 Apr 2015, Damien Lespiau <damien.lesp...@intel.com> wrote:
> On Fri, Apr 17, 2015 at 07:31:22PM +0300, Imre Deak wrote:
>> At the moment intel_prepare_ddi buffer will iterate through both MST and
>> CRT encoders, which is incorrect. Neither of these encoder types have an
>> embedding intel_digital_port object, so for these encoder types we will
>> use random data when dereferencing the corresponding
>> intel_digital_port->port field.
>> 
>> Introduced in
>> commit b403745c84592b26a0713e6944c2b109f6df5c82
>> Author: Damien Lespiau <damien.lesp...@intel.com>
>> Date:   Mon Aug 4 22:01:33 2014 +0100
>> 
>>     drm/i915: Iterate through the initialized DDIs to prepare their buffers
>> 
>> v2:
>> - fix getting at the port for MST encoders too
>> - make sure that intel_prepare_ddi_buffers() gets called for port E too
>>   (Paulo)
>> 
>> Signed-off-by: Imre Deak <imre.d...@intel.com>
>
> Reviewed-by: Damien Lespiau <damien.lesp...@intel.com>

Both pushed to drm-intel-next-queued, with the Bugzilla: tag moved to
2/2 which actually fixes the bug. Thanks for the patches and review.

BR,
Jani.


>
> -- 
> Damien
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  5 -----
>>  drivers/gpu/drm/i915/intel_ddi.c | 28 ++++++++++++++++++----------
>>  2 files changed, 18 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 40ef672..c461b56 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -251,11 +251,6 @@ enum hpd_pin {
>>                          &dev->mode_config.connector_list,   \
>>                          base.head)
>>  
>> -#define for_each_digital_port(dev, digital_port)            \
>> -    list_for_each_entry(digital_port,                       \
>> -                        &dev->mode_config.encoder_list,     \
>> -                        base.base.head)
>> -
>>  #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
>>      list_for_each_entry((intel_encoder), &(dev)->mode_config.encoder_list, 
>> base.head) \
>>              if ((intel_encoder)->base.crtc == (__crtc))
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 903d395..9c1e74a 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -256,12 +256,11 @@ intel_dig_port_supports_hdmi(const struct 
>> intel_digital_port *intel_dig_port)
>>   * in either FDI or DP modes only, as HDMI connections will work with both
>>   * of those
>>   */
>> -static void intel_prepare_ddi_buffers(struct drm_device *dev,
>> -                                  struct intel_digital_port *intel_dig_port)
>> +static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port 
>> port,
>> +                                  bool supports_hdmi)
>>  {
>>      struct drm_i915_private *dev_priv = dev->dev_private;
>>      u32 reg;
>> -    int port = intel_dig_port->port;
>>      int i, n_hdmi_entries, n_dp_entries, n_edp_entries, hdmi_default_entry,
>>          size;
>>      int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>> @@ -272,7 +271,7 @@ static void intel_prepare_ddi_buffers(struct drm_device 
>> *dev,
>>      const struct ddi_buf_trans *ddi_translations;
>>  
>>      if (IS_BROXTON(dev)) {
>> -            if (!intel_dig_port_supports_hdmi(intel_dig_port))
>> +            if (!supports_hdmi)
>>                      return;
>>  
>>              /* Vswing programming for HDMI */
>> @@ -360,7 +359,7 @@ static void intel_prepare_ddi_buffers(struct drm_device 
>> *dev,
>>              reg += 4;
>>      }
>>  
>> -    if (!intel_dig_port_supports_hdmi(intel_dig_port))
>> +    if (!supports_hdmi)
>>              return;
>>  
>>      /* Choose a good default if VBT is badly populated */
>> @@ -380,18 +379,27 @@ static void intel_prepare_ddi_buffers(struct 
>> drm_device *dev,
>>   */
>>  void intel_prepare_ddi(struct drm_device *dev)
>>  {
>> -    struct intel_digital_port *intel_dig_port;
>> +    struct intel_encoder *intel_encoder;
>>      bool visited[I915_MAX_PORTS] = { 0, };
>>  
>>      if (!HAS_DDI(dev))
>>              return;
>>  
>> -    for_each_digital_port(dev, intel_dig_port) {
>> -            if (visited[intel_dig_port->port])
>> +    for_each_intel_encoder(dev, intel_encoder) {
>> +            struct intel_digital_port *intel_dig_port;
>> +            enum port port;
>> +            bool supports_hdmi;
>> +
>> +            ddi_get_encoder_port(intel_encoder, &intel_dig_port, &port);
>> +
>> +            if (visited[port])
>>                      continue;
>>  
>> -            intel_prepare_ddi_buffers(dev, intel_dig_port);
>> -            visited[intel_dig_port->port] = true;
>> +            supports_hdmi = intel_dig_port &&
>> +                            intel_dig_port_supports_hdmi(intel_dig_port);
>> +
>> +            intel_prepare_ddi_buffers(dev, port, supports_hdmi);
>> +            visited[port] = true;
>>      }
>>  }
>>  
>> -- 
>> 2.1.0
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to