2013/12/5 Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>:
> On Thu, 21 Nov 2013, Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>>
>> When we call intel_dp_i2c_init we already get some I2C calls, which
>> will trigger a VDD enable, which will make use of the panel power
>> sequencing registers, so we need to have them ready by this time.
>
> But this patch won't make the registers ready either! The only thing
> this one does is initialize the delays in intel_dp. (And unlock the
> registers, but that's done in the vdd enable function too.)
Facepalm. You're right. I was mainly looking at the msleep() calls,
which are fixed by this patch. Besides this, we also need the
registers to be correct, but I guess this should be a separate patch.
>
> And you actually can't trivially initialize the registers either,
> because we will only later figure out whether this is a ghost eDP, and
> such initialization might botch up LVDS.
I wonder if we shouldn't at least try to do the right thing on HSW+,
since there's no LVDS there. But then there's the "eDP on port D" case
which we can't forget.
>
> See
> commit f30d26e468322b50d5e376bec40658683aff8ece
> Author: Jani Nikula <jani.nikula@xxxxxxxxx>
> Date: Wed Jan 16 10:53:40 2013 +0200
>
> drm/i915/eDP: do not write power sequence registers for ghost eDP
>
> I'm sorry I don't readily have any suggestions.
>
> If you are only interested in fixing the delays for msleep, then please
> fix the commit message!
Yeah, I'll fix the commit message for now, but I'll also think about
what to do with the registers.
Thanks,
Paulo
>
> BR,
> Jani.
>
>
>
>>
>> The good side is that we were reading the values, but were not using
>> them for anything (because we were just skipping the msleep(0) calls),
>> so this "fix" shouldn't fix any real existing bugs. I was only able to
>> identify the problem because I added some debug code to check how much
>> time time we were saving with my previous patch.
>>
>> Regression introduced by:
>> commit ed92f0b239ac971edc509169ae3d6955fbe0a188
>> Author: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> Date: Wed Jun 12 17:27:24 2013 -0300
>> drm/i915: extract intel_edp_init_connector
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 15 ++++++++-------
>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 3a1ca80..23927a0 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3565,14 +3565,14 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>> }
>>
>> static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> - struct intel_connector *intel_connector)
>> + struct intel_connector *intel_connector,
>> + struct edp_power_seq *power_seq)
>> {
>> struct drm_connector *connector = &intel_connector->base;
>> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> struct drm_device *dev = intel_dig_port->base.base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct drm_display_mode *fixed_mode = NULL;
>> - struct edp_power_seq power_seq = { 0 };
>> bool has_dpcd;
>> struct drm_display_mode *scan;
>> struct edid *edid;
>> @@ -3580,8 +3580,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> if (!is_edp(intel_dp))
>> return true;
>>
>> - intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
>> -
>> /* Cache DPCD and EDID for edp. */
>> ironlake_edp_panel_vdd_on(intel_dp);
>> has_dpcd = intel_dp_get_dpcd(intel_dp);
>> @@ -3599,8 +3597,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> }
>>
>> /* We now know it's not a ghost, init power sequence regs. */
>> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
>> - &power_seq);
>> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq);
>>
>> edid = drm_get_edid(connector, &intel_dp->adapter);
>> if (edid) {
>> @@ -3649,6 +3646,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> struct drm_device *dev = intel_encoder->base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> enum port port = intel_dig_port->port;
>> + struct edp_power_seq power_seq = { 0 };
>> const char *name = NULL;
>> int type, error;
>>
>> @@ -3748,13 +3746,16 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> BUG();
>> }
>>
>> + if (is_edp(intel_dp))
>> + intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
>> +
>> error = intel_dp_i2c_init(intel_dp, intel_connector, name);
>> WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
>> error, port_name(port));
>>
>> intel_dp->psr_setup_done = false;
>>
>> - if (!intel_edp_init_connector(intel_dp, intel_connector)) {
>> + if (!intel_edp_init_connector(intel_dp, intel_connector, &power_seq)) {
>> i2c_del_adapter(&intel_dp->adapter);
>> if (is_edp(intel_dp)) {
>> cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
