On Fri, Nov 02, 2018 at 11:06:43PM +0200, Souza, Jose wrote:
> On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> > Similarly to the GEN9_LP DPIO PHY code keep the CNL+ combo PHY code
> > in a
> > separate file.
> >
> > No functional change.
> >
> > Suggested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/Makefile | 1 +
> > drivers/gpu/drm/i915/i915_drv.h | 6 ++
> > drivers/gpu/drm/i915/intel_combo_phy.c | 159
> > ++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 127 ++------------------
> > -----
> > 4 files changed, 174 insertions(+), 119 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/intel_combo_phy.c
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile
> > index 28c7d7884e88..1e7e9513bb10 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -113,6 +113,7 @@ i915-y += intel_audio.o \
> > intel_bios.o \
> > intel_cdclk.o \
> > intel_color.o \
> > + intel_combo_phy.o \
> > intel_connector.o \
> > intel_display.o \
> > intel_dpio_phy.o \
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 6157f8128cc5..62882e1ddbee 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3571,6 +3571,12 @@ void vlv_phy_pre_encoder_enable(struct
> > intel_encoder *encoder,
> > void vlv_phy_reset_lanes(struct intel_encoder *encoder,
> > const struct intel_crtc_state
> > *old_crtc_state);
> >
> > +/* intel_combo_phy.c */
> > +void icl_combo_phys_init(struct drm_i915_private *dev_priv);
> > +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> > +void cnl_combo_phys_init(struct drm_i915_private *dev_priv);
> > +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> > +
> > int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
> > int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
> > u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> > b/drivers/gpu/drm/i915/intel_combo_phy.c
> > new file mode 100644
> > index 000000000000..13184ae5a217
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> > @@ -0,0 +1,159 @@
> > +/*
> > + * Copyright © 2018 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom
> > the
> > + * Software is furnished to do so, subject to the following
> > conditions:
> > + *
> > + * The above copyright notice and this permission notice (including
> > the next
> > + * paragraph) shall be included in all copies or substantial
> > portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> > EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> > OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER
> > + * DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#include "intel_drv.h"
> > +
> > +enum {
> > + PROCMON_0_85V_DOT_0,
> > + PROCMON_0_95V_DOT_0,
> > + PROCMON_0_95V_DOT_1,
> > + PROCMON_1_05V_DOT_0,
> > + PROCMON_1_05V_DOT_1,
> > +};
> > +
> > +static const struct cnl_procmon {
> > + u32 dw1, dw9, dw10;
> > +} cnl_procmon_values[] = {
> > + [PROCMON_0_85V_DOT_0] =
> > + { .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 =
> > 0x51914F96, },
> > + [PROCMON_0_95V_DOT_0] =
> > + { .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =
> > 0x77CA5EAB, },
> > + [PROCMON_0_95V_DOT_1] =
> > + { .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =
> > 0x8AE871C5, },
> > + [PROCMON_1_05V_DOT_0] =
> > + { .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =
> > 0x89E46DC1, },
> > + [PROCMON_1_05V_DOT_1] =
> > + { .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =
> > 0x8AE38FF1, },
> > +};
> > +
> > +/*
> > + * CNL has just one set of registers, while ICL has two sets: one
> > for port A and
> > + * the other for port B. The CNL registers are equivalent to the ICL
> > port A
> > + * registers, that's why we call the ICL macros even though the
> > function has CNL
> > + * on its name.
> > + */
> > +static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > + enum port port)
> > +{
> > + const struct cnl_procmon *procmon;
> > + u32 val;
> > +
> > + val = I915_READ(ICL_PORT_COMP_DW3(port));
> > + switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> > + default:
> > + MISSING_CASE(val);
> > + /* fall through */
> > + case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> > + procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> > + break;
> > + case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> > + procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> > + break;
> > + case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> > + procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> > + break;
> > + case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> > + procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> > + break;
> > + case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> > + procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> > + break;
> > + }
> > +
> > + val = I915_READ(ICL_PORT_COMP_DW1(port));
> > + val &= ~((0xff << 16) | 0xff);
> > + val |= procmon->dw1;
> > + I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> > +
> > + I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> > + I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> > +}
> > +
> > +void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
> > +{
> > + u32 val;
> > +
> > + val = I915_READ(CHICKEN_MISC_2);
> > + val &= ~CNL_COMP_PWR_DOWN;
> > + I915_WRITE(CHICKEN_MISC_2, val);
> > +
> > + /* Dummy PORT_A to get the correct CNL register from the ICL
> > macro */
> > + cnl_set_procmon_ref_values(dev_priv, PORT_A);
> > +
> > + val = I915_READ(CNL_PORT_COMP_DW0);
> > + val |= COMP_INIT;
> > + I915_WRITE(CNL_PORT_COMP_DW0, val);
> > +
> > + val = I915_READ(CNL_PORT_CL1CM_DW5);
> > + val |= CL_POWER_DOWN_ENABLE;
> > + I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> > +}
> > +
> > +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> > +{
> > + u32 val;
> > +
> > + val = I915_READ(CHICKEN_MISC_2);
> > + val |= CNL_COMP_PWR_DOWN;
> > + I915_WRITE(CHICKEN_MISC_2, val);
> > +}
> > +
> > +void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> > +{
> > + enum port port;
> > +
> > + for (port = PORT_A; port <= PORT_B; port++) {
> > + u32 val;
> > +
> > + val = I915_READ(ICL_PHY_MISC(port));
> > + val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > + I915_WRITE(ICL_PHY_MISC(port), val);
> > +
> > + cnl_set_procmon_ref_values(dev_priv, port);
> > +
> > + val = I915_READ(ICL_PORT_COMP_DW0(port));
> > + val |= COMP_INIT;
> > + I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > +
> > + val = I915_READ(ICL_PORT_CL_DW5(port));
> > + val |= CL_POWER_DOWN_ENABLE;
> > + I915_WRITE(ICL_PORT_CL_DW5(port), val);
> > + }
> > +}
> > +
> > +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> > +{
> > + enum port port;
> > +
> > + for (port = PORT_A; port <= PORT_B; port++) {
> > + u32 val;
> > +
> > + val = I915_READ(ICL_PHY_MISC(port));
> > + val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > + I915_WRITE(ICL_PHY_MISC(port), val);
> > +
> > + val = I915_READ(ICL_PORT_COMP_DW0(port));
> > + val &= ~COMP_INIT;
> > + I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > + }
> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index a7eea8423580..f8da471e81aa 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3436,99 +3436,18 @@ void bxt_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> > usleep_range(10, 30); /* 10 us delay per Bspec */
> > }
> >
> > -enum {
> > - PROCMON_0_85V_DOT_0,
> > - PROCMON_0_95V_DOT_0,
> > - PROCMON_0_95V_DOT_1,
> > - PROCMON_1_05V_DOT_0,
> > - PROCMON_1_05V_DOT_1,
> > -};
> > -
> > -static const struct cnl_procmon {
> > - u32 dw1, dw9, dw10;
> > -} cnl_procmon_values[] = {
> > - [PROCMON_0_85V_DOT_0] =
> > - { .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 =
> > 0x51914F96, },
> > - [PROCMON_0_95V_DOT_0] =
> > - { .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =
> > 0x77CA5EAB, },
> > - [PROCMON_0_95V_DOT_1] =
> > - { .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =
> > 0x8AE871C5, },
> > - [PROCMON_1_05V_DOT_0] =
> > - { .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =
> > 0x89E46DC1, },
> > - [PROCMON_1_05V_DOT_1] =
> > - { .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =
> > 0x8AE38FF1, },
> > -};
> > -
> > -/*
> > - * CNL has just one set of registers, while ICL has two sets: one
> > for port A and
> > - * the other for port B. The CNL registers are equivalent to the ICL
> > port A
> > - * registers, that's why we call the ICL macros even though the
> > function has CNL
> > - * on its name.
> > - */
> > -static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > - enum port port)
> > -{
> > - const struct cnl_procmon *procmon;
> > - u32 val;
> > -
> > - val = I915_READ(ICL_PORT_COMP_DW3(port));
> > - switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> > - default:
> > - MISSING_CASE(val);
> > - /* fall through */
> > - case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> > - procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> > - break;
> > - case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> > - procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> > - break;
> > - case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> > - procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> > - break;
> > - case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> > - procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> > - break;
> > - case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> > - procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> > - break;
> > - }
> > -
> > - val = I915_READ(ICL_PORT_COMP_DW1(port));
> > - val &= ~((0xff << 16) | 0xff);
> > - val |= procmon->dw1;
> > - I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> > -
> > - I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> > - I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> > -}
> > -
> > static void cnl_display_core_init(struct drm_i915_private *dev_priv,
> > bool resume)
> > {
> > struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> > struct i915_power_well *well;
> > - u32 val;
> >
> > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >
> > /* 1. Enable PCH Reset Handshake */
> > intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >
> > - /* 2. Enable Comp */
> > - val = I915_READ(CHICKEN_MISC_2);
> > - val &= ~CNL_COMP_PWR_DOWN;
> > - I915_WRITE(CHICKEN_MISC_2, val);
> > -
> > - /* Dummy PORT_A to get the correct CNL register from the ICL
> > macro */
> > - cnl_set_procmon_ref_values(dev_priv, PORT_A);
> > -
> > - val = I915_READ(CNL_PORT_COMP_DW0);
> > - val |= COMP_INIT;
> > - I915_WRITE(CNL_PORT_COMP_DW0, val);
> > -
> > - /* 3. */
> > - val = I915_READ(CNL_PORT_CL1CM_DW5);
> > - val |= CL_POWER_DOWN_ENABLE;
> > - I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> > + /* 2-3. */
> > + cnl_combo_phys_init(dev_priv);
> >
> > /*
> > * 4. Enable Power Well 1 (PG1).
> > @@ -3553,7 +3472,6 @@ static void cnl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> > {
> > struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> > struct i915_power_well *well;
> > - u32 val;
> >
> > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >
> > @@ -3577,10 +3495,8 @@ static void cnl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >
> > usleep_range(10, 30); /* 10 us delay per Bspec */
> >
> > - /* 5. Disable Comp */
> > - val = I915_READ(CHICKEN_MISC_2);
> > - val |= CNL_COMP_PWR_DOWN;
> > - I915_WRITE(CHICKEN_MISC_2, val);
> > + /* 5. */
> > + cnl_combo_phys_uninit(dev_priv);
> > }
> >
> > void icl_display_core_init(struct drm_i915_private *dev_priv,
> > @@ -3588,31 +3504,14 @@ void icl_display_core_init(struct
> > drm_i915_private *dev_priv,
> > {
> > struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> > struct i915_power_well *well;
> > - enum port port;
> > - u32 val;
> >
> > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >
> > /* 1. Enable PCH reset handshake. */
> > intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >
> > - for (port = PORT_A; port <= PORT_B; port++) {
> > - /* 2. Enable DDI combo PHY comp. */
> > - val = I915_READ(ICL_PHY_MISC(port));
> > - val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > - I915_WRITE(ICL_PHY_MISC(port), val);
> > -
> > - cnl_set_procmon_ref_values(dev_priv, port);
> > -
> > - val = I915_READ(ICL_PORT_COMP_DW0(port));
> > - val |= COMP_INIT;
> > - I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > -
> > - /* 3. Set power down enable. */
>
> If respining this patch, please consider also move the step comments to
> the new functions.
I don't find these comments very useful, I think they just say what the
code already expresses well and you can match that just fine against the
spec too.
>
> Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
>
>
> > - val = I915_READ(ICL_PORT_CL_DW5(port));
> > - val |= CL_POWER_DOWN_ENABLE;
> > - I915_WRITE(ICL_PORT_CL_DW5(port), val);
> > - }
> > + /* 2-3. */
> > + icl_combo_phys_init(dev_priv);
> >
> > /*
> > * 4. Enable Power Well 1 (PG1).
> > @@ -3640,8 +3539,6 @@ void icl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> > {
> > struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> > struct i915_power_well *well;
> > - enum port port;
> > - u32 val;
> >
> > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >
> > @@ -3663,16 +3560,8 @@ void icl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> > intel_power_well_disable(dev_priv, well);
> > mutex_unlock(&power_domains->lock);
> >
> > - /* 5. Disable Comp */
> > - for (port = PORT_A; port <= PORT_B; port++) {
> > - val = I915_READ(ICL_PHY_MISC(port));
> > - val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > - I915_WRITE(ICL_PHY_MISC(port), val);
> > -
> > - val = I915_READ(ICL_PORT_COMP_DW0(port));
> > - val &= ~COMP_INIT;
> > - I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > - }
> > + /* 5. */
> > + icl_combo_phys_uninit(dev_priv);
> > }
> >
> > static void chv_phy_control_init(struct drm_i915_private *dev_priv)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
