[Intel-gfx] [PATCH 1/2] drm/i915: move psr_setup_done to psr struct

[Rodrigo Vivi]

On Tue, Dec 17, 2013 at 6:06 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
> 2013/12/17 Rodrigo Vivi <rodrigo.vivi at gmail.com>:
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  | 1 +
>>  drivers/gpu/drm/i915/intel_dp.c  | 6 +++---
>>  drivers/gpu/drm/i915/intel_drv.h | 1 -
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 53288f6..cae3225 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -715,6 +715,7 @@ struct i915_fbc {
>>  struct i915_psr {
>>         bool sink_support;
>>         bool source_ok;
>> +       bool setup_done;
>>  };
>>
>>  enum intel_pch {
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 9b40113..f062a59 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1567,7 +1567,7 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
>>         struct drm_i915_private *dev_priv = dev->dev_private;
>>         struct edp_vsc_psr psr_vsc;
>>
>> -       if (intel_dp->psr_setup_done)
>> +       if (dev_priv->psr.setup_done)
>>                 return;
>>
>>         /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
>> @@ -1582,7 +1582,7 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
>>         I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
>>                    EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
>>
>> -       intel_dp->psr_setup_done = true;
>> +       dev_priv->psr.setup_done = true;
>>  }
>>
>>  static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
>> @@ -3702,7 +3702,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>>         WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
>>              error, port_name(port));
>>
>> -       intel_dp->psr_setup_done = false;
>> +       dev_priv->psr.setup_done = false;
>
> This line is now weird because most systems will call
> intel_dp_init_connector more than once, so we'll zero setup_done many
> times. I don't see any bugs related to this, but we should probably
> move this line to somewhere else.

yeah, it isn't a problem... some of my tests here I was making it
false on all disable call.
But it is better if it was only once for sure...
and I was using on dp_init what I guess doesn't help either, right?
so, do you suggest any place?

> Or just remove this line and rely on
> the fact that the struct is initialized as zero when allocated. It
> seems sink_support and source_ok are not explicitly initialized, so we
> should probably follow this model.

Can we trust that? If this is a sure thing I think this is might be
the best approach.

Thanks!
>
>
>>
>>         if (!intel_edp_init_connector(intel_dp, intel_connector)) {
>>                 i2c_del_adapter(&intel_dp->adapter);
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 46aea6c..f7b5b2f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -485,7 +485,6 @@ struct intel_dp {
>>         int backlight_off_delay;
>>         struct delayed_work panel_vdd_work;
>>         bool want_panel_vdd;
>> -       bool psr_setup_done;
>>         struct intel_connector *attached_connector;
>>  };
>>
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br