All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>
To: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>,
	"Nikula, Jani" <jani.nikula@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
Date: Thu, 1 Dec 2022 19:06:29 +0000	[thread overview]
Message-ID: <d431c48eae76fce327e8367e88ccfb5af528b484.camel@intel.com> (raw)
In-Reply-To: <87lens3jxh.fsf@intel.com>



> > > 
> > > Please let's avoid the ** here and everywhere.
> > > 
> > Alan: In order to to avoid causing the entire driver into a rebuild because of any change in the intel_pxp structure,
> > the only way to accomplish that is to use a ptr in i915. But using a ptr means we allocate the memory at init time and
> > free it at fini time and those 2 cases would require the ptr-to-ptr to ensure we get the correct store. The only way i
> > can avoid the ** is be passing i915 as the param and then populating the ptr via i915->pxp. Would this work?
> 
> In general, one of two approaches should be used:
> 
> 1) The caller takes care of storing/clearing the pointer:
> 
> struct intel_pxp *intel_pxp_init(void);
> void intel_pxp_fini(struct intel_pxp *pxp);
> 
> 2) The callee takes care of storing/clearing the pointer:
> 
> int intel_pxp_init(struct drm_i915_private *i915);
> void intel_pxp_fini(struct drm_i915_private *i915);
> 
> Passing pointers to pointers is not as clean.
> 
> In this case, I'd choose 2) just because it's being called at the high
> level, and it's pretty much in line with everything else.
> 
> 
> 
> BR,
> Jani.

I like option 2. Also, I just remembered I used a similiar approach for guc-error-capture  (where intel_guc_capture_init
received a ptr to 'struct intel_guc' which owns a 'struct intel_guc_state_capture *capture;' member and did the
allocation). I did that back then exactly to address Jani's concerns because i was tired of waiting on build times when
developing offline :D

Will respin accordingly.

....alan


  reply	other threads:[~2022-12-01 19:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-29  0:31 [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Alan Previn
2022-11-29  0:31 ` [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915 Alan Previn
2022-11-29 18:35   ` Teres Alexis, Alan Previn
2022-11-29 21:28   ` Rodrigo Vivi
2022-11-30  0:10     ` Teres Alexis, Alan Previn
2022-11-30  8:50       ` Jani Nikula
2022-12-01 19:06         ` Teres Alexis, Alan Previn [this message]
2022-12-01 23:45     ` Teres Alexis, Alan Previn
2022-11-29  0:51 ` [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Teres Alexis, Alan Previn
2022-11-29  0:53 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
2022-11-29  1:07 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-11-29 21:29 ` [Intel-gfx] [PATCH v5 0/1] " Rodrigo Vivi
2022-11-30  0:35   ` Teres Alexis, Alan Previn

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=d431c48eae76fce327e8367e88ccfb5af528b484.camel@intel.com \
    --to=alan.previn.teres.alexis@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@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.