On Tue, Dec 17, 2013 at 6:06 PM, Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> 2013/12/17 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
