Re: [PATCH 62/89] drm/i915/skl: Query DPLL attached to port on SKL

["M, Satheeshakrishna" <satheeshakrishna.m@intel.com>]

On 9/23/2014 1:54 AM, Paulo Zanoni wrote:
> 2014-09-04 8:27 GMT-03:00 Damien Lespiau<damien.lespiau@intel.com>:
>> From: Satheeshakrishna M<satheeshakrishna.m@intel.com>
>>
>> Modify the implementation to query DPLL attached to a SKL port.
>>
>> v2: Rebase on top of the run-time PM on DPMS series (Damien)
>>
>> Signed-off-by: Satheeshakrishna M<satheeshakrishna.m@intel.com>
>> Signed-off-by: Damien Lespiau<damien.lespiau@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++++-
>>   drivers/gpu/drm/i915/intel_drv.h     |  5 ++++-
>>   2 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 69e023a..6e71250 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7789,6 +7789,28 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>>          return 0;
>>   }
>>
>> +static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv,
>> +                               enum port port,
>> +                               struct intel_crtc_config *pipe_config)
>> +{
>> +       u32 temp;
>> +
>> +       temp = I915_READ(DPLL_CTRL2) & DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
>> +       pipe_config->ddi_pll_sel = temp >> (port * 3 + 1);
> Since we're relying on the fact that ddi_pll_sel is actually "enum
> skl_dpll", I'd change the definition of the enum and explicitly
> declare SKL_DPLL0 to be "0", and SKL_DPLL1 to be 1, and so on.
Makes sense
>> +
>> +       switch (pipe_config->ddi_pll_sel) {
>> +       case SKL_DPLL1:
>> +               pipe_config->shared_dpll = DPLL_ID_SKL_DPLL1;
>> +               break;
>> +       case SKL_DPLL2:
>> +               pipe_config->shared_dpll = DPLL_ID_SKL_DPLL2;
>> +               break;
>> +       case SKL_DPLL3:
>> +               pipe_config->shared_dpll = DPLL_ID_SKL_DPLL3;
>> +               break;
> Please also add a WARN() to the default case, especially since there
> are 4 possible values and we're just checking 3.
ok
>> +       }
>> +}
>> +
>>   static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
>>                                  enum port port,
>>                                  struct intel_crtc_config *pipe_config)
>> @@ -7818,7 +7840,10 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>>
>>          port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
>>
>> -       haswell_get_ddi_pll(dev_priv, port, pipe_config);
>> +       if (IS_SKYLAKE(dev))
> If you invert the check so that SKL is on the "else" part (as you did
> in your previous patch), you'll make sure we're ready for the next
> Gens in case they happen to be same as SKL. It's more likely they will
> be the same as SKL instead of being the same as HSW :)
>
>
> With at least the WARN added on the switch statement: Reviewed-by:
> Paulo Zanoni<paulo.r.zanoni@intel.com>
>
>> +               skylake_get_ddi_pll(dev_priv, port, pipe_config);
>> +       else
>> +               haswell_get_ddi_pll(dev_priv, port, pipe_config);
>>
>>          if (pipe_config->shared_dpll >= 0) {
>>                  pll = &dev_priv->shared_dplls[pipe_config->shared_dpll];
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 559b747..9558f07 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -323,7 +323,10 @@ struct intel_crtc_config {
>>          /* Selected dpll when shared or DPLL_ID_PRIVATE. */
>>          enum intel_dpll_id shared_dpll;
>>
>> -       /* PORT_CLK_SEL for DDI ports. */
>> +       /*
>> +        * - PORT_CLK_SEL for DDI ports on HSW/BDW.
>> +        * - enum skl_dpll on SKL
>> +        */
>>          uint32_t ddi_pll_sel;
>>
>>          /* Actual register state of the dpll, for shared dpll cross-checking. */
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>