public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Andi Shyti <andi.shyti@linux.intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: MichalłWiniarski <michal.winiarski@intel.com>,
	"Intel GFX" <intel-gfx@lists.freedesktop.org>,
	"DRI Devel" <dri-devel@lists.freedesktop.org>,
	"Chris Wilson" <chris@chris-wilson.co.uk>
Subject: Re: [Intel-gfx] [PATCH v4 2/2] drm/i915: Use to_root_gt() to refer to the root tile
Date: Wed, 1 Dec 2021 00:41:08 +0200	[thread overview]
Message-ID: <YaaohLkiuWsPSqWq@intel.intel> (raw)
In-Reply-To: <20211130210730.wbuy3znor6jel3cc@ldmartin-desk2>

Hi Lucas,

fist of all thanks for taking a look at this, I was eagerly
waiting for reviewers.

On Tue, Nov 30, 2021 at 01:07:30PM -0800, Lucas De Marchi wrote:
> On Sun, Nov 28, 2021 at 01:09:26PM +0200, Andi Shyti wrote:
> > Starting from a patch from Matt to_root_gt() returns the
> > reference to the root tile in order to abstract the root tile
> > from th callers.
> > 
> > Being the root tile identified as tile '0', embed the id in the
> > name so that i915->gt becomes i915->gt0.
> > 
> > The renaming has been mostly done with the following command and
> > some manual fixes.
> > 
> > sed -i -e sed -i 's/\&i915\->gt\./\&to_root_gt(i915)\->/g' \
> > 	-e sed -i 's/\&dev_priv\->gt\./\&to_root_gt(dev_priv)\->/g' \
> > 	-e 's/\&dev_priv\->gt/to_root_gt(dev_priv)/g' \
> > 	-e 's/\&i915\->gt/to_root_gt(i915)/g' \
> > 	-e 's/dev_priv\->gt\./to_root_gt(dev_priv)\->/g' \
> > 	-e 's/i915\->gt\./to_root_gt(i915)\->/g' \
> > 	`find drivers/gpu/drm/i915/ -name *.[ch]`
> > 
> > Two small changes have been added to this commit:
> > 
> > 1. intel_reset_gpu() in intel_display.c retreives the gt from
> >    to_scanout_gt()
> > 2. in set_scheduler_caps() the gt is taken from the engine and
> >    not from i915.
> 
> Ideally the non-automatic changes should be in separate patches, before
> the ones that can be done by automation. Because then it becomes easier
> to apply the final result without conflicts.

OK

> This is quite a big diff to merge in one go. Looking at the pending
> patches from Michal however I see he had similar changes, split in
> sensible chunks..  Could you split your version like that? at least
> gt/gem and display would be good to have separate. Or sync with Michal
> on how to proceed with these versions Here are his patches:
> 
> 	drm/i915: Remove i915->ggtt
> 	drm/i915: Use to_gt() helper for GGTT accesses
> 	drm/i915: Use to_gt() helper
> 	drm/i915/gvt: Use to_gt() helper
> 	drm/i915/gem: Use to_gt() helper
> 	drm/i915/gt: Use to_gt() helper
> 	drm/i915/display: Use to_gt() helper
> 	drm/i915: Introduce to_gt() helper

I understand... will follow this approach.

> This first patch also removed the `struct intel_gt *gt = to_gt(pool)`,
> that would otherwise be a leftover in drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c

One difference from Michal patch is that I am not using the
wrapper

  to_gt(...)

but

  to_root_gt(...)

which was introduced by Matt. To me sounds more meaningful as it
specifies that we are really looking for the root tile and not
any tile.

Thanks again,
Andi

  reply	other threads:[~2021-11-30 22:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-28 11:09 [Intel-gfx] [PATCH v4 0/2] More preparation for multi gt patches Andi Shyti
2021-11-28 11:09 ` [Intel-gfx] [PATCH v4 1/2] drm/i915: Store backpointer to GT in uncore Andi Shyti
2021-11-30 23:52   ` Andi Shyti
2021-11-28 11:09 ` [Intel-gfx] [PATCH v4 2/2] drm/i915: Use to_root_gt() to refer to the root tile Andi Shyti
2021-11-30 21:07   ` Lucas De Marchi
2021-11-30 22:41     ` Andi Shyti [this message]
2021-12-01  0:38       ` Lucas De Marchi
2021-12-01  9:44         ` Michal Wajdeczko
2021-12-01 12:58           ` Andi Shyti
2021-11-28 11:38 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for More preparation for multi gt patches (rev4) Patchwork
2021-11-28 12:11 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-11-28 13:58 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " 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=YaaohLkiuWsPSqWq@intel.intel \
    --to=andi.shyti@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=michal.winiarski@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