From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Oscar Mateo <oscar.mateo@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v4 03/13] drm/i915/guc: Add onion teardown to the GuC setup
Date: Mon, 27 Mar 2017 16:06:48 +0300 [thread overview]
Message-ID: <1490620008.3166.27.camel@linux.intel.com> (raw)
In-Reply-To: <20170324085954.GA7560@nuc-i3427.alporthouse.com>
On pe, 2017-03-24 at 08:59 +0000, Chris Wilson wrote:
> On Thu, Mar 23, 2017 at 09:36:10AM -0700, Oscar Mateo wrote:
> >
> > On 03/23/2017 03:57 PM, Chris Wilson wrote:
> > >
> > > I'm not happy with moving subfeature detection logic into the core GEM
> > > code. if (i915.enable_guc_loading) firstly should never be a module
> > > parameter (it's derived state!) and secondly it should reside next to
> > > the dependent logic and not be interrupting the central control flow.
> > What do you mean it's derived state? from what?
>
> The set of features, whether to use guc submission, huc, or whether
> there is a platform requirement to load the firmware, define whether or
> not we need to request and upload a particular firmware. Every module
> option should be a quirk to alter driver behaviour (i.e. a debugging
> crutch), few and strongly justified (we have too many) and necessarily
> global in scope. Device specific options should ideally use a more
> specific interface (most clear examples are the panel specific quirks).
Misguidance here was probably me enforcing the view that by hoisting
and unifying the checks, it'll be easier to comprehend the code when a
check covers greater amount of lines. I have to agree that the top-
level driver control flow should not be having the checks, but each
code path existing only within a .c file should have the "skip this
branch" check earlier than later.
Sorry for misguidance :P
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-03-27 13:06 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-22 17:39 [PATCH 00/13] Various improvements around the GuC topic Oscar Mateo
2017-03-22 17:39 ` [PATCH v5 01/13] drm/i915/guc: Sanitize GuC client initialization Oscar Mateo
2017-03-22 17:39 ` [PATCH v5 02/13] drm/i915/guc: Keep the ctx_pool_vaddr mapped, for easy access Oscar Mateo
2017-03-22 17:39 ` [PATCH v4 03/13] drm/i915/guc: Add onion teardown to the GuC setup Oscar Mateo
2017-03-23 22:57 ` Chris Wilson
2017-03-23 16:36 ` Oscar Mateo
2017-03-24 8:59 ` Chris Wilson
2017-03-27 13:06 ` Joonas Lahtinen [this message]
2017-03-27 17:33 ` Oscar Mateo
2017-03-22 17:39 ` [PATCH 04/13] drm/i915/guc: The Additional Data Struct (ADS) should get enabled together with GuC submission Oscar Mateo
2017-03-22 17:39 ` [PATCH v3 05/13] drm/i915/guc: Break out the GuC log extras into their own "runtime" struct Oscar Mateo
2017-03-22 17:39 ` [PATCH v3 06/13] drm/i915/guc: Make intel_guc_send a function pointer Oscar Mateo
2017-03-22 17:39 ` [PATCH v2 07/13] drm/i915/guc: Improve the GuC documentation & comments about proxy submissions Oscar Mateo
2017-03-22 17:39 ` [PATCH v2 08/13] drm/i915/guc: Wait for doorbell to be inactive before deallocating Oscar Mateo
2017-03-22 17:39 ` [PATCH v2 09/13] drm/i915/guc: A little bit more of doorbell sanitization Oscar Mateo
2017-03-22 17:39 ` [PATCH v3 10/13] drm/i915/guc: Refactor the concept "GuC context descriptor" into "GuC stage descriptor" Oscar Mateo
2017-03-22 17:39 ` [PATCH 11/13] drm/i915/guc: Split out the mmio_white_list struct Oscar Mateo
2017-03-22 17:39 ` [PATCH 12/13] drm/i915/guc: Move guc_interrupts_release next to guc_interrupts_capture Oscar Mateo
2017-03-22 17:39 ` [PATCH 13/13] HAX Enable GuC loading & submission Oscar Mateo
2017-03-23 11:06 ` ✓ Fi.CI.BAT: success for Various improvements around the GuC topic Patchwork
2017-03-23 13:20 ` Joonas Lahtinen
2017-03-23 15:15 ` Oscar Mateo
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=1490620008.3166.27.camel@linux.intel.com \
--to=joonas.lahtinen@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=oscar.mateo@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox