diff mbox series

[4/4] drm/i915: Drop WaIncreaseLatencyIPCEnabled/1140 for cnl

Message ID 20190131074216.3994-4-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/4] drm/i915: Enable transition watermarks for glk | expand

Commit Message

Ville Syrjälä Jan. 31, 2019, 7:42 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Drop WaIncreaseLatencyIPCEnabled/Display w/a #1140 for
early cnl steppings. Also switch the kbl/cfl case to check
for IS_GEN9_BC() for brevity. It ends up being the same thing
because IPC is disabled on SKL due to w/a #0477.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Rodrigo Vivi Jan. 31, 2019, 6:10 p.m. UTC | #1 
On Thu, Jan 31, 2019 at 09:42:16AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Drop WaIncreaseLatencyIPCEnabled/Display w/a #1140 for
> early cnl steppings. Also switch the kbl/cfl case to check
> for IS_GEN9_BC() for brevity. It ends up being the same thing
> because IPC is disabled on SKL due to w/a #0477.

I think this deserves a commend in the code, otherwise someone
in the future might not notice that and send a patch to replace
9_BC per KBL || CFL...

anyway:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 306e41ccc50e..55491e2d5134 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4701,9 +4701,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
>  	 * WaIncreaseLatencyIPCEnabled: kbl,cfl
>  	 * Display WA #1141: kbl,cfl
>  	 */
> -	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> -	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> -	    dev_priv->ipc_enabled)
> +	if (IS_GEN9_BC(dev_priv) && dev_priv->ipc_enabled)
>  		latency += 4;
>  
>  	if (skl_needs_memory_bw_wa(dev_priv) && wp->x_tiled)
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Feb. 4, 2019, 5:40 p.m. UTC | #2 
On Thu, Jan 31, 2019 at 10:10:47AM -0800, Rodrigo Vivi wrote:
> On Thu, Jan 31, 2019 at 09:42:16AM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Drop WaIncreaseLatencyIPCEnabled/Display w/a #1140 for
> > early cnl steppings. Also switch the kbl/cfl case to check
> > for IS_GEN9_BC() for brevity. It ends up being the same thing
> > because IPC is disabled on SKL due to w/a #0477.
> 
> I think this deserves a commend in the code, otherwise someone
> in the future might not notice that and send a patch to replace
> 9_BC per KBL || CFL...
> 
> anyway:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Ta. I just noticed there were some other IS_KBL||IS_CFL cases in the
code as well. So maybe I'll just leave it as is here too.

One thing I don't like is that w/a #0477 is in the device info. I think
I'll want to move that into ipc code to make it less confusing what's
going on.

> 
> 
> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 306e41ccc50e..55491e2d5134 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4701,9 +4701,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
> >  	 * WaIncreaseLatencyIPCEnabled: kbl,cfl
> >  	 * Display WA #1141: kbl,cfl
> >  	 */
> > -	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> > -	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> > -	    dev_priv->ipc_enabled)
> > +	if (IS_GEN9_BC(dev_priv) && dev_priv->ipc_enabled)
> >  		latency += 4;
> >  
> >  	if (skl_needs_memory_bw_wa(dev_priv) && wp->x_tiled)
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi Feb. 4, 2019, 5:49 p.m. UTC | #3 
On Mon, Feb 04, 2019 at 07:40:56PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 31, 2019 at 10:10:47AM -0800, Rodrigo Vivi wrote:
> > On Thu, Jan 31, 2019 at 09:42:16AM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Drop WaIncreaseLatencyIPCEnabled/Display w/a #1140 for
> > > early cnl steppings. Also switch the kbl/cfl case to check
> > > for IS_GEN9_BC() for brevity. It ends up being the same thing
> > > because IPC is disabled on SKL due to w/a #0477.
> > 
> > I think this deserves a commend in the code, otherwise someone
> > in the future might not notice that and send a patch to replace
> > 9_BC per KBL || CFL...
> > 
> > anyway:
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Ta. I just noticed there were some other IS_KBL||IS_CFL cases in the
> code as well. So maybe I'll just leave it as is here too.
> 
> One thing I don't like is that w/a #0477 is in the device info. I think
> I'll want to move that into ipc code to make it less confusing what's
> going on.


yeap, I think it makes more sense to have the wa inside ipc code rather
than inside device info. But on the other hand it gets confuse if we
end up reading has_ipc=1 and then we have to go inside ipc to find
out that actually has_ipc is lying :/

> 
> > 
> > 
> > 
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 306e41ccc50e..55491e2d5134 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -4701,9 +4701,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
> > >  	 * WaIncreaseLatencyIPCEnabled: kbl,cfl
> > >  	 * Display WA #1141: kbl,cfl
> > >  	 */
> > > -	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> > > -	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> > > -	    dev_priv->ipc_enabled)
> > > +	if (IS_GEN9_BC(dev_priv) && dev_priv->ipc_enabled)
> > >  		latency += 4;
> > >  
> > >  	if (skl_needs_memory_bw_wa(dev_priv) && wp->x_tiled)
> > > -- 
> > > 2.19.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Feb. 4, 2019, 5:58 p.m. UTC | #4 
On Mon, Feb 04, 2019 at 09:49:42AM -0800, Rodrigo Vivi wrote:
> On Mon, Feb 04, 2019 at 07:40:56PM +0200, Ville Syrjälä wrote:
> > On Thu, Jan 31, 2019 at 10:10:47AM -0800, Rodrigo Vivi wrote:
> > > On Thu, Jan 31, 2019 at 09:42:16AM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Drop WaIncreaseLatencyIPCEnabled/Display w/a #1140 for
> > > > early cnl steppings. Also switch the kbl/cfl case to check
> > > > for IS_GEN9_BC() for brevity. It ends up being the same thing
> > > > because IPC is disabled on SKL due to w/a #0477.
> > > 
> > > I think this deserves a commend in the code, otherwise someone
> > > in the future might not notice that and send a patch to replace
> > > 9_BC per KBL || CFL...
> > > 
> > > anyway:
> > > 
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > Ta. I just noticed there were some other IS_KBL||IS_CFL cases in the
> > code as well. So maybe I'll just leave it as is here too.
> > 
> > One thing I don't like is that w/a #0477 is in the device info. I think
> > I'll want to move that into ipc code to make it less confusing what's
> > going on.
> 
> 
> yeap, I think it makes more sense to have the wa inside ipc code rather
> than inside device info. But on the other hand it gets confuse if we
> end up reading has_ipc=1 and then we have to go inside ipc to find
> out that actually has_ipc is lying :/

It's not really lying though. The hw has it, we're just not supposed
to use it.

> 
> > 
> > > 
> > > 
> > > 
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_pm.c | 4 +---
> > > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > > index 306e41ccc50e..55491e2d5134 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > @@ -4701,9 +4701,7 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
> > > >  	 * WaIncreaseLatencyIPCEnabled: kbl,cfl
> > > >  	 * Display WA #1141: kbl,cfl
> > > >  	 */
> > > > -	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> > > > -	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
> > > > -	    dev_priv->ipc_enabled)
> > > > +	if (IS_GEN9_BC(dev_priv) && dev_priv->ipc_enabled)
> > > >  		latency += 4;
> > > >  
> > > >  	if (skl_needs_memory_bw_wa(dev_priv) && wp->x_tiled)
> > > > -- 
> > > > 2.19.2
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 306e41ccc50e..55491e2d5134 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4701,9 +4701,7 @@  static void skl_compute_plane_wm(const struct intel_crtc_state *cstate,
 	 * WaIncreaseLatencyIPCEnabled: kbl,cfl
 	 * Display WA #1141: kbl,cfl
 	 */
-	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
-	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
-	    dev_priv->ipc_enabled)
+	if (IS_GEN9_BC(dev_priv) && dev_priv->ipc_enabled)
 		latency += 4;
 
 	if (skl_needs_memory_bw_wa(dev_priv) && wp->x_tiled)