All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org,
	Lucas De Marchi <lucas.demarchi@intel.com>
Subject: Re: [PATCH] drm/i915: Sort ctx workarounds init from newer to older platforms.
Date: Mon, 25 Feb 2019 14:59:27 +0200	[thread overview]
Message-ID: <87h8csrplc.fsf@intel.com> (raw)
In-Reply-To: <20190222212509.GF2855@intel.com>

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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-02-25 12:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-21 23:14 [PATCH] drm/i915: Sort ctx workarounds init from newer to older platforms Rodrigo Vivi
2019-02-22  2:01 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-02-22 10:32 ` [PATCH] " Chris Wilson
2019-02-22 21:25   ` Rodrigo Vivi
2019-02-25 12:59     ` Jani Nikula [this message]
2019-02-22 16:26 ` ✓ Fi.CI.IGT: success for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h8csrplc.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.