Re: [Intel-gfx] [PATCH v2 10/22] drm/i915/rkl: RKL only uses PHY_MISC for PHY's A and B

2020-05-07 Thread Srivatsa, Anusha



> -Original Message-
> From: Roper, Matthew D 
> Sent: Wednesday, May 6, 2020 10:20 PM
> To: Srivatsa, Anusha 
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2 10/22] drm/i915/rkl: RKL only uses PHY_MISC
> for PHY's A and B
> 
> On Wed, May 06, 2020 at 06:49:06AM -0700, Srivatsa, Anusha wrote:
> >
> >
> > > -Original Message- From: Intel-gfx
> > >  On Behalf Of Matt Roper
> > > Sent: Tuesday, May 5, 2020 4:22 AM To:
> > > intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v2
> > > 10/22] drm/i915/rkl: RKL only uses PHY_MISC for PHY's A and B
> > >
> > > Since the number of platforms with this restriction are growing,
> > > let's separate out the platform logic into a has_phy_misc()
> > > function.
> > >
> > > Bspec: 50107 Signed-off-by: Matt Roper 
> > > --- .../gpu/drm/i915/display/intel_combo_phy.c| 30
> > > +++ 1 file changed, 17 insertions(+), 13
> > > deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> > > b/drivers/gpu/drm/i915/display/intel_combo_phy.c index
> > > 9ff05ec12115..43d8784f6fa0 100644 ---
> > > a/drivers/gpu/drm/i915/display/intel_combo_phy.c +++
> > > b/drivers/gpu/drm/i915/display/intel_combo_phy.c @@ -181,11 +181,25
> > > @@ static void cnl_combo_phys_uninit(struct drm_i915_private
> > > *dev_priv) intel_de_write(dev_priv, CHICKEN_MISC_2, val);  }
> > >
> > > +static bool has_phy_misc(struct drm_i915_private *i915, enum phy
> > > phy) { +  /* + * Some platforms only expect PHY_MISC to be
> > > programmed for PHY-A and + * PHY-B and may not even have
> instances
> > > of the register for the +  * other combo PHY's.  + */ + if
> > > (IS_ELKHARTLAKE(i915) || +IS_ROCKETLAKE(i915)) + return phy <
> > > PHY_C;
> > According BSpec 50107, there is an instance of this for combo PHY C as
> > well.
> >
> Yeah, there's technically an instance of the register, but the only field in 
> it that
> our driver programs has a RKL programming note that says "This register field
> need only be programmed for port A and B."

Ok. Thanks for pointing it out.

Reviewed-by: Anusha Srivatsa 

> 
> Matt
> 
> > Anusha
> > > +
> > > + return true;
> > > +}
> > > +
> > >  static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
> > > enum phy phy)
> > >  {
> > >   /* The PHY C added by EHL has no PHY_MISC register */
> > > - if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> > > + if (!has_phy_misc(dev_priv, phy))
> > >   return intel_de_read(dev_priv, ICL_PORT_COMP_DW0(phy))
> &
> > > COMP_INIT;
> > >   else
> > >   return !(intel_de_read(dev_priv, ICL_PHY_MISC(phy)) & @@ -
> > > 317,12 +331,7 @@ static void icl_combo_phys_init(struct
> > > drm_i915_private
> > > *dev_priv)
> > >   continue;
> > >   }
> > >
> > > - /*
> > > -  * Although EHL adds a combo PHY C, there's no PHY_MISC
> > > -  * register for it and no need to program the
> > > -  * DE_IO_COMP_PWR_DOWN setting on PHY C.
> > > -  */
> > > - if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> > > + if (!has_phy_misc(dev_priv, phy))
> > >   goto skip_phy_misc;
> > >
> > >   /*
> > > @@ -376,12 +385,7 @@ static void icl_combo_phys_uninit(struct
> > > drm_i915_private *dev_priv)
> > >"Combo PHY %c HW state changed
> unexpectedly\n",
> > >phy_name(phy));
> > >
> > > - /*
> > > -  * Although EHL adds a combo PHY C, there's no PHY_MISC
> > > -  * register for it and no need to program the
> > > -  * DE_IO_COMP_PWR_DOWN setting on PHY C.
> > > -  */
> > > - if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> > > + if (!has_phy_misc(dev_priv, phy))
> > >   goto skip_phy_misc;
> > >
> > >   val = intel_de_read(dev_priv, ICL_PHY_MISC(phy));
> > > --
> > > 2.24.1
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 10/22] drm/i915/rkl: RKL only uses PHY_MISC for PHY's A and B

2020-05-06 Thread Matt Roper
On Wed, May 06, 2020 at 06:49:06AM -0700, Srivatsa, Anusha wrote:
> 
> 
> > -Original Message- From: Intel-gfx
> >  On Behalf Of Matt Roper
> > Sent: Tuesday, May 5, 2020 4:22 AM To:
> > intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v2
> > 10/22] drm/i915/rkl: RKL only uses PHY_MISC for PHY's A and B
> > 
> > Since the number of platforms with this restriction are growing,
> > let's separate out the platform logic into a has_phy_misc()
> > function.
> > 
> > Bspec: 50107 Signed-off-by: Matt Roper 
> > --- .../gpu/drm/i915/display/intel_combo_phy.c| 30
> > +++ 1 file changed, 17 insertions(+), 13
> > deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> > b/drivers/gpu/drm/i915/display/intel_combo_phy.c index
> > 9ff05ec12115..43d8784f6fa0 100644 ---
> > a/drivers/gpu/drm/i915/display/intel_combo_phy.c +++
> > b/drivers/gpu/drm/i915/display/intel_combo_phy.c @@ -181,11 +181,25
> > @@ static void cnl_combo_phys_uninit(struct drm_i915_private
> > *dev_priv) intel_de_write(dev_priv, CHICKEN_MISC_2, val);  }
> > 
> > +static bool has_phy_misc(struct drm_i915_private *i915, enum phy
> > phy) { +/* + * Some platforms only expect PHY_MISC to be
> > programmed for PHY-A and +   * PHY-B and may not even have instances
> > of the register for the +* other combo PHY's.  + */ + if
> > (IS_ELKHARTLAKE(i915) || +  IS_ROCKETLAKE(i915)) + return phy <
> > PHY_C;
> According BSpec 50107, there is an instance of this for combo PHY C as
> well. 
> 
Yeah, there's technically an instance of the register, but the only
field in it that our driver programs has a RKL programming note that
says "This register field need only be programmed for port A and B."


Matt

> Anusha
> > +
> > +   return true;
> > +}
> > +
> >  static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
> >   enum phy phy)
> >  {
> > /* The PHY C added by EHL has no PHY_MISC register */
> > -   if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> > +   if (!has_phy_misc(dev_priv, phy))
> > return intel_de_read(dev_priv, ICL_PORT_COMP_DW0(phy))
> > & COMP_INIT;
> > else
> > return !(intel_de_read(dev_priv, ICL_PHY_MISC(phy)) & @@ -
> > 317,12 +331,7 @@ static void icl_combo_phys_init(struct drm_i915_private
> > *dev_priv)
> > continue;
> > }
> > 
> > -   /*
> > -* Although EHL adds a combo PHY C, there's no PHY_MISC
> > -* register for it and no need to program the
> > -* DE_IO_COMP_PWR_DOWN setting on PHY C.
> > -*/
> > -   if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> > +   if (!has_phy_misc(dev_priv, phy))
> > goto skip_phy_misc;
> > 
> > /*
> > @@ -376,12 +385,7 @@ static void icl_combo_phys_uninit(struct
> > drm_i915_private *dev_priv)
> >  "Combo PHY %c HW state changed
> > unexpectedly\n",
> >  phy_name(phy));
> > 
> > -   /*
> > -* Although EHL adds a combo PHY C, there's no PHY_MISC
> > -* register for it and no need to program the
> > -* DE_IO_COMP_PWR_DOWN setting on PHY C.
> > -*/
> > -   if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> > +   if (!has_phy_misc(dev_priv, phy))
> > goto skip_phy_misc;
> > 
> > val = intel_de_read(dev_priv, ICL_PHY_MISC(phy));
> > --
> > 2.24.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 10/22] drm/i915/rkl: RKL only uses PHY_MISC for PHY's A and B

2020-05-06 Thread Srivatsa, Anusha



> -Original Message-
> From: Intel-gfx  On Behalf Of Matt
> Roper
> Sent: Tuesday, May 5, 2020 4:22 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 10/22] drm/i915/rkl: RKL only uses PHY_MISC for
> PHY's A and B
> 
> Since the number of platforms with this restriction are growing, let's 
> separate
> out the platform logic into a has_phy_misc() function.
> 
> Bspec: 50107
> Signed-off-by: Matt Roper 
> ---
>  .../gpu/drm/i915/display/intel_combo_phy.c| 30 +++
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> index 9ff05ec12115..43d8784f6fa0 100644
> --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> @@ -181,11 +181,25 @@ static void cnl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>   intel_de_write(dev_priv, CHICKEN_MISC_2, val);  }
> 
> +static bool has_phy_misc(struct drm_i915_private *i915, enum phy phy) {
> + /*
> +  * Some platforms only expect PHY_MISC to be programmed for PHY-A
> and
> +  * PHY-B and may not even have instances of the register for the
> +  * other combo PHY's.
> +  */
> + if (IS_ELKHARTLAKE(i915) ||
> + IS_ROCKETLAKE(i915))
> + return phy < PHY_C;
According BSpec 50107, there is an instance of this for combo PHY C as well. 

Anusha
> +
> + return true;
> +}
> +
>  static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
> enum phy phy)
>  {
>   /* The PHY C added by EHL has no PHY_MISC register */
> - if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> + if (!has_phy_misc(dev_priv, phy))
>   return intel_de_read(dev_priv, ICL_PORT_COMP_DW0(phy))
> & COMP_INIT;
>   else
>   return !(intel_de_read(dev_priv, ICL_PHY_MISC(phy)) & @@ -
> 317,12 +331,7 @@ static void icl_combo_phys_init(struct drm_i915_private
> *dev_priv)
>   continue;
>   }
> 
> - /*
> -  * Although EHL adds a combo PHY C, there's no PHY_MISC
> -  * register for it and no need to program the
> -  * DE_IO_COMP_PWR_DOWN setting on PHY C.
> -  */
> - if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> + if (!has_phy_misc(dev_priv, phy))
>   goto skip_phy_misc;
> 
>   /*
> @@ -376,12 +385,7 @@ static void icl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>"Combo PHY %c HW state changed
> unexpectedly\n",
>phy_name(phy));
> 
> - /*
> -  * Although EHL adds a combo PHY C, there's no PHY_MISC
> -  * register for it and no need to program the
> -  * DE_IO_COMP_PWR_DOWN setting on PHY C.
> -  */
> - if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
> + if (!has_phy_misc(dev_priv, phy))
>   goto skip_phy_misc;
> 
>   val = intel_de_read(dev_priv, ICL_PHY_MISC(phy));
> --
> 2.24.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2 10/22] drm/i915/rkl: RKL only uses PHY_MISC for PHY's A and B

2020-05-04 Thread Matt Roper
Since the number of platforms with this restriction are growing, let's
separate out the platform logic into a has_phy_misc() function.

Bspec: 50107
Signed-off-by: Matt Roper 
---
 .../gpu/drm/i915/display/intel_combo_phy.c| 30 +++
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c 
b/drivers/gpu/drm/i915/display/intel_combo_phy.c
index 9ff05ec12115..43d8784f6fa0 100644
--- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
@@ -181,11 +181,25 @@ static void cnl_combo_phys_uninit(struct drm_i915_private 
*dev_priv)
intel_de_write(dev_priv, CHICKEN_MISC_2, val);
 }
 
+static bool has_phy_misc(struct drm_i915_private *i915, enum phy phy)
+{
+   /*
+* Some platforms only expect PHY_MISC to be programmed for PHY-A and
+* PHY-B and may not even have instances of the register for the
+* other combo PHY's.
+*/
+   if (IS_ELKHARTLAKE(i915) ||
+   IS_ROCKETLAKE(i915))
+   return phy < PHY_C;
+
+   return true;
+}
+
 static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
  enum phy phy)
 {
/* The PHY C added by EHL has no PHY_MISC register */
-   if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
+   if (!has_phy_misc(dev_priv, phy))
return intel_de_read(dev_priv, ICL_PORT_COMP_DW0(phy)) & 
COMP_INIT;
else
return !(intel_de_read(dev_priv, ICL_PHY_MISC(phy)) &
@@ -317,12 +331,7 @@ static void icl_combo_phys_init(struct drm_i915_private 
*dev_priv)
continue;
}
 
-   /*
-* Although EHL adds a combo PHY C, there's no PHY_MISC
-* register for it and no need to program the
-* DE_IO_COMP_PWR_DOWN setting on PHY C.
-*/
-   if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
+   if (!has_phy_misc(dev_priv, phy))
goto skip_phy_misc;
 
/*
@@ -376,12 +385,7 @@ static void icl_combo_phys_uninit(struct drm_i915_private 
*dev_priv)
 "Combo PHY %c HW state changed unexpectedly\n",
 phy_name(phy));
 
-   /*
-* Although EHL adds a combo PHY C, there's no PHY_MISC
-* register for it and no need to program the
-* DE_IO_COMP_PWR_DOWN setting on PHY C.
-*/
-   if (IS_ELKHARTLAKE(dev_priv) && phy == PHY_C)
+   if (!has_phy_misc(dev_priv, phy))
goto skip_phy_misc;
 
val = intel_de_read(dev_priv, ICL_PHY_MISC(phy));
-- 
2.24.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx