diff mbox series

drm/i915: Sort ctx workarounds init from newer to older platforms.

Message ID 20190221231452.21672-1-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Sort ctx workarounds init from newer to older platforms. | expand

Commit Message

Rodrigo Vivi Feb. 21, 2019, 11:14 p.m. UTC
No functional change. Just a reorg to match the preferred
behavior.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_workarounds.c | 36 ++++++++++++------------
 1 file changed, 18 insertions(+), 18 deletions(-)

Comments

Chris Wilson Feb. 22, 2019, 10:32 a.m. UTC | #1
Quoting Rodrigo Vivi (2019-02-21 23:14:52)
> No functional change. Just a reorg to match the preferred
> behavior.
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_workarounds.c | 36 ++++++++++++------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index 15f4a6dee5aa..743cf5b00155 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -564,26 +564,26 @@ void intel_engine_init_ctx_wa(struct intel_engine_cs *engine)
>  
>         wa_init_start(wal, "context");
>  
> -       if (INTEL_GEN(i915) < 8)
> -               return;
> -       else if (IS_BROADWELL(i915))
> -               bdw_ctx_workarounds_init(engine);
> -       else if (IS_CHERRYVIEW(i915))
> -               chv_ctx_workarounds_init(engine);
> -       else if (IS_SKYLAKE(i915))
> -               skl_ctx_workarounds_init(engine);
> -       else if (IS_BROXTON(i915))
> -               bxt_ctx_workarounds_init(engine);
> -       else if (IS_KABYLAKE(i915))
> -               kbl_ctx_workarounds_init(engine);
> -       else if (IS_GEMINILAKE(i915))
> -               glk_ctx_workarounds_init(engine);
> -       else if (IS_COFFEELAKE(i915))
> -               cfl_ctx_workarounds_init(engine);

If the chains start with

if (0)
	/* space left intentionally blank */
else if (IS_ICELAKE(i915))

adding more to the chain later would have less collateral damage.

> +       if (IS_ICELAKE(i915))
> +               icl_ctx_workarounds_init(engine);
>         else if (IS_CANNONLAKE(i915))
>                 cnl_ctx_workarounds_init(engine);
> -       else if (IS_ICELAKE(i915))
> -               icl_ctx_workarounds_init(engine);
> +       else if (IS_COFFEELAKE(i915))
> +               cfl_ctx_workarounds_init(engine);
> +       else if (IS_GEMINILAKE(i915))
> +               glk_ctx_workarounds_init(engine);
> +       else if (IS_KABYLAKE(i915))
> +               kbl_ctx_workarounds_init(engine);
> +       else if (IS_BROXTON(i915))
> +               bxt_ctx_workarounds_init(engine);
> +       else if (IS_SKYLAKE(i915))
> +               skl_ctx_workarounds_init(engine);
> +       else if (IS_CHERRYVIEW(i915))
> +               chv_ctx_workarounds_init(engine);
> +       else if (IS_BROADWELL(i915))
> +               bdw_ctx_workarounds_init(engine);

For the sake of consistency,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Rodrigo Vivi Feb. 22, 2019, 9:25 p.m. UTC | #2
On Fri, Feb 22, 2019 at 10:32:25AM +0000, Chris Wilson wrote:
> Quoting Rodrigo Vivi (2019-02-21 23:14:52)
> > No functional change. Just a reorg to match the preferred
> > behavior.
> > 
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_workarounds.c | 36 ++++++++++++------------
> >  1 file changed, 18 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> > index 15f4a6dee5aa..743cf5b00155 100644
> > --- a/drivers/gpu/drm/i915/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> > @@ -564,26 +564,26 @@ void intel_engine_init_ctx_wa(struct intel_engine_cs *engine)
> >  
> >         wa_init_start(wal, "context");
> >  
> > -       if (INTEL_GEN(i915) < 8)
> > -               return;
> > -       else if (IS_BROADWELL(i915))
> > -               bdw_ctx_workarounds_init(engine);
> > -       else if (IS_CHERRYVIEW(i915))
> > -               chv_ctx_workarounds_init(engine);
> > -       else if (IS_SKYLAKE(i915))
> > -               skl_ctx_workarounds_init(engine);
> > -       else if (IS_BROXTON(i915))
> > -               bxt_ctx_workarounds_init(engine);
> > -       else if (IS_KABYLAKE(i915))
> > -               kbl_ctx_workarounds_init(engine);
> > -       else if (IS_GEMINILAKE(i915))
> > -               glk_ctx_workarounds_init(engine);
> > -       else if (IS_COFFEELAKE(i915))
> > -               cfl_ctx_workarounds_init(engine);
> 
> If the chains start with
> 
> if (0)
> 	/* space left intentionally blank */
> else if (IS_ICELAKE(i915))
> 
> adding more to the chain later would have less collateral damage.

interesting idea... I will consider, but for now
let's move with the consistency and similar approach
everywhere.

> 
> > +       if (IS_ICELAKE(i915))
> > +               icl_ctx_workarounds_init(engine);
> >         else if (IS_CANNONLAKE(i915))
> >                 cnl_ctx_workarounds_init(engine);
> > -       else if (IS_ICELAKE(i915))
> > -               icl_ctx_workarounds_init(engine);
> > +       else if (IS_COFFEELAKE(i915))
> > +               cfl_ctx_workarounds_init(engine);
> > +       else if (IS_GEMINILAKE(i915))
> > +               glk_ctx_workarounds_init(engine);
> > +       else if (IS_KABYLAKE(i915))
> > +               kbl_ctx_workarounds_init(engine);
> > +       else if (IS_BROXTON(i915))
> > +               bxt_ctx_workarounds_init(engine);
> > +       else if (IS_SKYLAKE(i915))
> > +               skl_ctx_workarounds_init(engine);
> > +       else if (IS_CHERRYVIEW(i915))
> > +               chv_ctx_workarounds_init(engine);
> > +       else if (IS_BROADWELL(i915))
> > +               bdw_ctx_workarounds_init(engine);
> 
> For the sake of consistency,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

pushed to dinq

Thanks,
Rodrigo.

> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Feb. 25, 2019, 12:59 p.m. UTC | #3
On Fri, 22 Feb 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Fri, Feb 22, 2019 at 10:32:25AM +0000, Chris Wilson wrote:
>> Quoting Rodrigo Vivi (2019-02-21 23:14:52)
>> > No functional change. Just a reorg to match the preferred
>> > behavior.
>> > 
>> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_workarounds.c | 36 ++++++++++++------------
>> >  1 file changed, 18 insertions(+), 18 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
>> > index 15f4a6dee5aa..743cf5b00155 100644
>> > --- a/drivers/gpu/drm/i915/intel_workarounds.c
>> > +++ b/drivers/gpu/drm/i915/intel_workarounds.c
>> > @@ -564,26 +564,26 @@ void intel_engine_init_ctx_wa(struct intel_engine_cs *engine)
>> >  
>> >         wa_init_start(wal, "context");
>> >  
>> > -       if (INTEL_GEN(i915) < 8)
>> > -               return;
>> > -       else if (IS_BROADWELL(i915))
>> > -               bdw_ctx_workarounds_init(engine);
>> > -       else if (IS_CHERRYVIEW(i915))
>> > -               chv_ctx_workarounds_init(engine);
>> > -       else if (IS_SKYLAKE(i915))
>> > -               skl_ctx_workarounds_init(engine);
>> > -       else if (IS_BROXTON(i915))
>> > -               bxt_ctx_workarounds_init(engine);
>> > -       else if (IS_KABYLAKE(i915))
>> > -               kbl_ctx_workarounds_init(engine);
>> > -       else if (IS_GEMINILAKE(i915))
>> > -               glk_ctx_workarounds_init(engine);
>> > -       else if (IS_COFFEELAKE(i915))
>> > -               cfl_ctx_workarounds_init(engine);
>> 
>> If the chains start with
>> 
>> if (0)
>> 	/* space left intentionally blank */
>> else if (IS_ICELAKE(i915))
>> 
>> adding more to the chain later would have less collateral damage.
>
> interesting idea... I will consider, but for now
> let's move with the consistency and similar approach
> everywhere.

I actually wondered if we've made a mistake sorting all if ladders
newest to oldest. Having the newest platform in the else branch would
solve a lot of problems. But too much churn now.

BR,
Jani.

>
>> 
>> > +       if (IS_ICELAKE(i915))
>> > +               icl_ctx_workarounds_init(engine);
>> >         else if (IS_CANNONLAKE(i915))
>> >                 cnl_ctx_workarounds_init(engine);
>> > -       else if (IS_ICELAKE(i915))
>> > -               icl_ctx_workarounds_init(engine);
>> > +       else if (IS_COFFEELAKE(i915))
>> > +               cfl_ctx_workarounds_init(engine);
>> > +       else if (IS_GEMINILAKE(i915))
>> > +               glk_ctx_workarounds_init(engine);
>> > +       else if (IS_KABYLAKE(i915))
>> > +               kbl_ctx_workarounds_init(engine);
>> > +       else if (IS_BROXTON(i915))
>> > +               bxt_ctx_workarounds_init(engine);
>> > +       else if (IS_SKYLAKE(i915))
>> > +               skl_ctx_workarounds_init(engine);
>> > +       else if (IS_CHERRYVIEW(i915))
>> > +               chv_ctx_workarounds_init(engine);
>> > +       else if (IS_BROADWELL(i915))
>> > +               bdw_ctx_workarounds_init(engine);
>> 
>> For the sake of consistency,
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> pushed to dinq
>
> Thanks,
> Rodrigo.
>
>> -Chris
>> _______________________________________________
>> 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
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 15f4a6dee5aa..743cf5b00155 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -564,26 +564,26 @@  void intel_engine_init_ctx_wa(struct intel_engine_cs *engine)
 
 	wa_init_start(wal, "context");
 
-	if (INTEL_GEN(i915) < 8)
-		return;
-	else if (IS_BROADWELL(i915))
-		bdw_ctx_workarounds_init(engine);
-	else if (IS_CHERRYVIEW(i915))
-		chv_ctx_workarounds_init(engine);
-	else if (IS_SKYLAKE(i915))
-		skl_ctx_workarounds_init(engine);
-	else if (IS_BROXTON(i915))
-		bxt_ctx_workarounds_init(engine);
-	else if (IS_KABYLAKE(i915))
-		kbl_ctx_workarounds_init(engine);
-	else if (IS_GEMINILAKE(i915))
-		glk_ctx_workarounds_init(engine);
-	else if (IS_COFFEELAKE(i915))
-		cfl_ctx_workarounds_init(engine);
+	if (IS_ICELAKE(i915))
+		icl_ctx_workarounds_init(engine);
 	else if (IS_CANNONLAKE(i915))
 		cnl_ctx_workarounds_init(engine);
-	else if (IS_ICELAKE(i915))
-		icl_ctx_workarounds_init(engine);
+	else if (IS_COFFEELAKE(i915))
+		cfl_ctx_workarounds_init(engine);
+	else if (IS_GEMINILAKE(i915))
+		glk_ctx_workarounds_init(engine);
+	else if (IS_KABYLAKE(i915))
+		kbl_ctx_workarounds_init(engine);
+	else if (IS_BROXTON(i915))
+		bxt_ctx_workarounds_init(engine);
+	else if (IS_SKYLAKE(i915))
+		skl_ctx_workarounds_init(engine);
+	else if (IS_CHERRYVIEW(i915))
+		chv_ctx_workarounds_init(engine);
+	else if (IS_BROADWELL(i915))
+		bdw_ctx_workarounds_init(engine);
+	else if (INTEL_GEN(i915) < 8)
+		return;
 	else
 		MISSING_CASE(INTEL_GEN(i915));