Re: [Intel-gfx] [PATCH v6 2/8] drm/i915/bxt: IPC WA for Broxton

[Lankhorst, Maarten]

Hey,

Mahesh Kumar schreef op ma 28-11-2016 om 18:37 [+0530]:
> Hi,
> 
> Will keep WA number in commit message/WA location.
> thanks,

Sounds good, with that fixed patches 1-5 and 7 look good to me.
I think patch 6 will no longer be required since the workaround status
will also be kept inside intel_state.

~Maarten
-
Intel International B.V.
Registered in The Netherlands under number 34098535
Statutory seat: Haarlemmermeer
Registered address: Capronilaan 37, 1119NG Schiphol-Rijk

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v6 2/8] drm/i915/bxt: IPC WA for Broxton

[Mahesh Kumar]

Hi,

Will keep WA number in commit message/WA location.
thanks,

Regards,

-Mahesh

On Thursday 24 November 2016 06:21 PM, Lankhorst, Maarten wrote:

Mahesh Kumar schreef op do 24-11-2016 om 09:31 [+0530]:

If IPC is enabled in BXT, display underruns are observed.
WA: The Line Time programmed in the WM_LINETIME register should be
half of the actual calculated Line Time.

Programmed Line Time = 1/2*Calculated Line Time

Signed-off-by: Mahesh Kumar 
Reviewed-by: Paulo Zanoni 
---
  drivers/gpu/drm/i915/i915_drv.c |  2 ++
  drivers/gpu/drm/i915/i915_drv.h |  2 ++
  drivers/gpu/drm/i915/intel_pm.c | 11 +--
  3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 445fec9..1b0a589 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1243,6 +1243,8 @@ int i915_driver_load(struct pci_dev *pdev,
const struct pci_device_id *ent)
  
  	intel_runtime_pm_enable(dev_priv);
  
+	dev_priv->ipc_enabled = false;

+
/* Everything is in place, we can now relax! */
DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
 driver.name, driver.major, driver.minor,
driver.patchlevel,
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 63c0ea0..6691a4e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2128,6 +2128,8 @@ struct drm_i915_private {
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
  
+	bool ipc_enabled;

+
/* Used to save the pipe-to-encoder mapping for audio */
struct intel_encoder *av_enc_map[I915_MAX_PIPES];
  
diff --git a/drivers/gpu/drm/i915/intel_pm.c

b/drivers/gpu/drm/i915/intel_pm.c
index 8908736..7090a7c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3769,7 +3769,10 @@ skl_compute_wm_level(const struct
drm_i915_private *dev_priv,
  static uint32_t
  skl_compute_linetime_wm(struct intel_crtc_state *cstate)
  {
+   struct drm_atomic_state *state = cstate->base.state;
+   struct drm_i915_private *dev_priv = to_i915(state->dev);
uint32_t pixel_rate;
+   uint32_t linetime_wm;
  
  	if (!cstate->base.active)

return 0;
@@ -3779,8 +3782,12 @@ skl_compute_linetime_wm(struct
intel_crtc_state *cstate)
if (WARN_ON(pixel_rate == 0))
return 0;
  
-	return DIV_ROUND_UP(8 * cstate-

base.adjusted_mode.crtc_htotal * 1000,

-   pixel_rate);
+   linetime_wm = DIV_ROUND_UP(8 * cstate-

base.adjusted_mode.crtc_htotal *

+   1000, pixel_rate);
+   if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled)
+   linetime_wm = DIV_ROUND_UP(linetime_wm, 2);

I've asked on irc to be sure, but this needs slightly more info in the
code as well. Same for 3/8 and 8/8.

12:47 < mlankhorst> danvet: if we add hw workarounds that don't have a
name, do we still have to write comments about them in the code?
12:48 < danvet> at least a bspec reference
12:48 < danvet> some hint to make it possible to find where it's from
12:49 < danvet> e.g. mesa has traditionally just pasted the entire
bspec paragraph
12:49 < mlankhorst> ok
12:49 < danvet> we've done that too in the past, but then the wa db
mostly made that redundant
12:49 < danvet> but if there's no wa entry, then full deal
12:49 < danvet> also, maybe a bspec bug notice to pls add one
12:49 < danvet> you should be able to do that over the web, and afaik
there should be one for every w/a


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

Re: [Intel-gfx] [PATCH v6 2/8] drm/i915/bxt: IPC WA for Broxton

[Lankhorst, Maarten]

Mahesh Kumar schreef op do 24-11-2016 om 09:31 [+0530]:
> If IPC is enabled in BXT, display underruns are observed.
> WA: The Line Time programmed in the WM_LINETIME register should be
> half of the actual calculated Line Time.
> 
> Programmed Line Time = 1/2*Calculated Line Time
> 
> Signed-off-by: Mahesh Kumar 
> Reviewed-by: Paulo Zanoni 
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c | 11 +--
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 445fec9..1b0a589 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1243,6 +1243,8 @@ int i915_driver_load(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>  
>   intel_runtime_pm_enable(dev_priv);
>  
> + dev_priv->ipc_enabled = false;
> +
>   /* Everything is in place, we can now relax! */
>   DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>    driver.name, driver.major, driver.minor,
> driver.patchlevel,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 63c0ea0..6691a4e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2128,6 +2128,8 @@ struct drm_i915_private {
>   /* perform PHY state sanity checks? */
>   bool chv_phy_assert[2];
>  
> + bool ipc_enabled;
> +
>   /* Used to save the pipe-to-encoder mapping for audio */
>   struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 8908736..7090a7c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3769,7 +3769,10 @@ skl_compute_wm_level(const struct
> drm_i915_private *dev_priv,
>  static uint32_t
>  skl_compute_linetime_wm(struct intel_crtc_state *cstate)
>  {
> + struct drm_atomic_state *state = cstate->base.state;
> + struct drm_i915_private *dev_priv = to_i915(state->dev);
>   uint32_t pixel_rate;
> + uint32_t linetime_wm;
>  
>   if (!cstate->base.active)
>   return 0;
> @@ -3779,8 +3782,12 @@ skl_compute_linetime_wm(struct
> intel_crtc_state *cstate)
>   if (WARN_ON(pixel_rate == 0))
>   return 0;
>  
> - return DIV_ROUND_UP(8 * cstate-
> >base.adjusted_mode.crtc_htotal * 1000,
> - pixel_rate);
> + linetime_wm = DIV_ROUND_UP(8 * cstate-
> >base.adjusted_mode.crtc_htotal *
> + 1000, pixel_rate);
> + if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled)
> + linetime_wm = DIV_ROUND_UP(linetime_wm, 2);
I've asked on irc to be sure, but this needs slightly more info in the
code as well. Same for 3/8 and 8/8.

12:47 < mlankhorst> danvet: if we add hw workarounds that don't have a
name, do we still have to write comments about them in the code?
12:48 < danvet> at least a bspec reference
12:48 < danvet> some hint to make it possible to find where it's from
12:49 < danvet> e.g. mesa has traditionally just pasted the entire
bspec paragraph
12:49 < mlankhorst> ok
12:49 < danvet> we've done that too in the past, but then the wa db
mostly made that redundant
12:49 < danvet> but if there's no wa entry, then full deal
12:49 < danvet> also, maybe a bspec bug notice to pls add one
12:49 < danvet> you should be able to do that over the web, and afaik
there should be one for every w/a
-
Intel International B.V.
Registered in The Netherlands under number 34098535
Statutory seat: Haarlemmermeer
Registered address: Capronilaan 37, 1119NG Schiphol-Rijk

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx