[Intel-gfx] [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated

[Shankar, Uma]

>> -----Original Message-----
>> From: Kulkarni, Vandita
>> Sent: Thursday, March 21, 2019 7:23 PM
>> To: Shankar, Uma <uma.shankar at intel.com>;
>> intel-gfx at lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula at intel.com>; Chauhan, Madhav
>> <madhav.chauhan at intel.com>
>> Subject: RE: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated
>>
>>
>> > -----Original Message-----
>> > From: Shankar, Uma
>> > Sent: Wednesday, March 20, 2019 5:19 PM
>> > To: Kulkarni, Vandita <vandita.kulkarni at intel.com>; intel-
>> > gfx at lists.freedesktop.org
>> > Cc: Nikula, Jani <jani.nikula at intel.com>; Chauhan, Madhav
>> > <madhav.chauhan at intel.com>
>> > Subject: RE: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated
>> >
>> >
>> >
>> > >-----Original Message-----
>> > >From: Kulkarni, Vandita
>> > >Sent: Wednesday, March 20, 2019 3:39 PM
>> > >To: intel-gfx at lists.freedesktop.org
>> > >Cc: Nikula, Jani <jani.nikula at intel.com>; Shankar, Uma
>> > ><uma.shankar at intel.com>; Chauhan, Madhav
>> <madhav.chauhan at intel.com>;
>> > >Kulkarni, Vandita <vandita.kulkarni at intel.com>
>> > >Subject: [PATCH 1/2] drm/i915/icl/dsi: Ungate clocks if gated
>> >
>> > You can drop dsi from commit header. Just drm/i915/icl/ should be good.
>> > Also update header as Ungate ddi clocks if gated
>> Okay.
>> >
>> > >
>> > >IO enable sequencing needs ddi clocks enabled.
>> > >These clocks will be gated at the later point in the enable sequence.
>> > >
>> > >Signed-off-by: Vandita Kulkarni <vandita.kulkarni at intel.com>
>> > >---
>> > > drivers/gpu/drm/i915/icl_dsi.c | 8 ++++++++
>> > > 1 file changed, 8 insertions(+)
>> > >
>> > >diff --git a/drivers/gpu/drm/i915/icl_dsi.c
>> > >b/drivers/gpu/drm/i915/icl_dsi.c index beb30d9..f02504d 100644
>> > >--- a/drivers/gpu/drm/i915/icl_dsi.c
>> > >+++ b/drivers/gpu/drm/i915/icl_dsi.c
>> > >@@ -589,6 +589,14 @@ static void gen11_dsi_map_pll(struct
>> > >intel_encoder *encoder,
>> > > 		val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port);
>> > > 	}
>> > > 	I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>> > >+
>> > >+	/* make sure that the ddi clocks are not gated */
>> > >+	val = I915_READ(DPCLKA_CFGCR0_ICL);
>> > >+	for_each_dsi_port(port, intel_dsi->ports) {
>> > >+		val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port);
>> > >+	}
>> > >+	I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>> > >+
>> > > 	POSTING_READ(DPCLKA_CFGCR0_ICL);
>> >
>> > I think you can reuse the val from top and avoid an extra write to
>> > the same register.
>> At this point we ideally have the clocks gated and we need to ungate
>> it. We must write to this register.
>> Accordingly, will fix the commit header too.
>As per the spec, 2 different writes are needed for mapping and turning on the clocks.

Oh ok, yes seems like indeed spec expects separate write for individual steps.
Thanks for the pointing out. You can keep the current change and my RB.

Regards,
Uma Shankar

>Thanks,
>Vandita
>
>>
>> Thanks.
>> Vandita
>> >
>> > Otherwise change looks ok to me. With above comments fixed,
>> > Reviewed-by: Uma Shankar <uma.shankar at intel.com>
>> >
>> > >
>> > > 	mutex_unlock(&dev_priv->dpll_lock);
>> > >--
>> > >1.9.1