All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chery, Nanley G" <nanley.g.chery@intel.com>
To: "Deak, Imre" <imre.deak@intel.com>,
	"Chris Wilson" <chris@chris-wilson.co.uk>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Subject: RE: [Intel-gfx] [PATCH 1/2] drm/framebuffer: Format modifier for Intel Gen 12 render compression with Clear Color
Date: Fri, 11 Dec 2020 07:04:02 +0000	[thread overview]
Message-ID: <ac393a9d1b774766a35c299915f0cca5@intel.com> (raw)
In-Reply-To: <20201201120456.GC2849269@ideak-desk.fi.intel.com>



> -----Original Message-----
> From: Imre Deak <imre.deak@intel.com>
> Sent: Tuesday, December 1, 2020 4:05 AM
> To: Chery, Nanley G <nanley.g.chery@intel.com>; Chris Wilson <chris@chris-
> wilson.co.uk>; Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>; intel-gfx@lists.freedesktop.org; Nikula,
> Jani <jani.nikula@intel.com>; Daniel Vetter <daniel.vetter@ffwll.ch>;
> Kondapally, Kalyan <kalyan.kondapally@intel.com>; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; dri-devel@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/2] drm/framebuffer: Format modifier for
> Intel Gen 12 render compression with Clear Color
> 
> Hi Nanley,
> 
> thanks for the review.
> 
> +Ville, Chris.
> 
> On Tue, Dec 01, 2020 at 02:18:26AM +0200, Chery, Nanley G wrote:
> > Hi Imre,
> >
> > I have a question and a couple comments:
> >
> > Is the map of the clear color address creating a new synchronization
> > point between the GPU and CPU? If so, I wonder how this will impact
> > performance.
> 
> The kmap to read the clear value is not adding any sync overhead if
> that's what you mean. But the clear value must be in place before we
> read it out and that should be guaranteed by the flush we do anyway to wait
> for the render result (even considering the explicit L3/RT flush, depth
> stall the spec requires for fast clears).
> 
> However now that you mention: atm the kmap/readout happens after the
> explicit but before the implicit fence-wait. I think it should happen
> after the implicit fence-wait.
> 
> Ville, Chris, could you confirm the above and also that the above flush
> is enough to ensure the CPU read is coherent?
> 
> > There was some talk of asynchronously updating the clear color
> > register a while back.
> 
> Couldn't find anything with a quick search, do you have a pointer? Just
> before the flip we must wait for the render results anyway, as we do
> now, so not sure how it could be optimized.
> 
 
There were some offline discussions, so I don't have a reference unfortunately.
Though, given what you shared above it seems like it's actually not an issue.

> > We probably don't have to update the header, but we noticed in our
> > testing that the clear color prefers an alignment greater than 64B.
> > Unfortunately, I can't find any bspec note about this. As long as the
> > buffer creators are aware though, I think we should be fine. I don't
> > know if this is the best forum to bring it up, but I thought I'd
> > share.
> 
> Yes, would be good to clarify this and get it also to the spec. Then the
> driver should also check the alignment of the 3rd FB plane.
> 

I plan to run some more tests and file a bug in the spec.

I see that the IGT test only clears the fb once. Just to confirm, is the 
clear color offset read from on every frame? Userspace would like to be 
able to pass different clear colors for an fb. 

-Nanley

> > Seems like the upper converted clear color is untested due to the lack
> > of RGBX16 support. I suppose that if there are any issues there, they
> > can be fixed later...
> 
> Yes, a 64bpp RC-CC subtest in IGT is missing, should be easy to add
> that.
> 
> --Imre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Chery, Nanley G" <nanley.g.chery@intel.com>
To: "Deak, Imre" <imre.deak@intel.com>,
	"Chris Wilson" <chris@chris-wilson.co.uk>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/framebuffer: Format modifier for Intel Gen 12 render compression with Clear Color
Date: Fri, 11 Dec 2020 07:04:02 +0000	[thread overview]
Message-ID: <ac393a9d1b774766a35c299915f0cca5@intel.com> (raw)
In-Reply-To: <20201201120456.GC2849269@ideak-desk.fi.intel.com>



> -----Original Message-----
> From: Imre Deak <imre.deak@intel.com>
> Sent: Tuesday, December 1, 2020 4:05 AM
> To: Chery, Nanley G <nanley.g.chery@intel.com>; Chris Wilson <chris@chris-
> wilson.co.uk>; Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>; intel-gfx@lists.freedesktop.org; Nikula,
> Jani <jani.nikula@intel.com>; Daniel Vetter <daniel.vetter@ffwll.ch>;
> Kondapally, Kalyan <kalyan.kondapally@intel.com>; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; dri-devel@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/2] drm/framebuffer: Format modifier for
> Intel Gen 12 render compression with Clear Color
> 
> Hi Nanley,
> 
> thanks for the review.
> 
> +Ville, Chris.
> 
> On Tue, Dec 01, 2020 at 02:18:26AM +0200, Chery, Nanley G wrote:
> > Hi Imre,
> >
> > I have a question and a couple comments:
> >
> > Is the map of the clear color address creating a new synchronization
> > point between the GPU and CPU? If so, I wonder how this will impact
> > performance.
> 
> The kmap to read the clear value is not adding any sync overhead if
> that's what you mean. But the clear value must be in place before we
> read it out and that should be guaranteed by the flush we do anyway to wait
> for the render result (even considering the explicit L3/RT flush, depth
> stall the spec requires for fast clears).
> 
> However now that you mention: atm the kmap/readout happens after the
> explicit but before the implicit fence-wait. I think it should happen
> after the implicit fence-wait.
> 
> Ville, Chris, could you confirm the above and also that the above flush
> is enough to ensure the CPU read is coherent?
> 
> > There was some talk of asynchronously updating the clear color
> > register a while back.
> 
> Couldn't find anything with a quick search, do you have a pointer? Just
> before the flip we must wait for the render results anyway, as we do
> now, so not sure how it could be optimized.
> 
 
There were some offline discussions, so I don't have a reference unfortunately.
Though, given what you shared above it seems like it's actually not an issue.

> > We probably don't have to update the header, but we noticed in our
> > testing that the clear color prefers an alignment greater than 64B.
> > Unfortunately, I can't find any bspec note about this. As long as the
> > buffer creators are aware though, I think we should be fine. I don't
> > know if this is the best forum to bring it up, but I thought I'd
> > share.
> 
> Yes, would be good to clarify this and get it also to the spec. Then the
> driver should also check the alignment of the 3rd FB plane.
> 

I plan to run some more tests and file a bug in the spec.

I see that the IGT test only clears the fb once. Just to confirm, is the 
clear color offset read from on every frame? Userspace would like to be 
able to pass different clear colors for an fb. 

-Nanley

> > Seems like the upper converted clear color is untested due to the lack
> > of RGBX16 support. I suppose that if there are any issues there, they
> > can be fixed later...
> 
> Yes, a 64bpp RC-CC subtest in IGT is missing, should be easy to add
> that.
> 
> --Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-12-11  7:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-23 18:26 [Intel-gfx] [PATCH 1/2] drm/framebuffer: Format modifier for Intel Gen 12 render compression with Clear Color Imre Deak
2020-11-23 18:26 ` [Intel-gfx] [PATCH 2/2] drm/i915/tgl: Add Clear Color support for TGL Render Decompression Imre Deak
2020-11-27  9:27   ` Kahola, Mika
2020-12-01 12:34   ` Chris Wilson
2020-12-01 20:50     ` Imre Deak
2020-12-01 21:10       ` Chris Wilson
2020-12-01 21:31         ` Imre Deak
2020-11-23 20:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/framebuffer: Format modifier for Intel Gen 12 render compression with Clear Color Patchwork
2020-11-23 20:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-11-23 21:24 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-11-24 15:12   ` Imre Deak
2020-11-24 17:15     ` Vudum, Lakshminarayana
2020-11-24 17:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-11-24 21:57 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2020-11-26  8:24 ` [Intel-gfx] [PATCH 1/2] " Kahola, Mika
2020-11-27 14:31 ` Imre Deak
2020-11-27 14:31   ` Imre Deak
2020-11-27 15:19   ` Daniel Vetter
2020-11-27 15:19     ` Daniel Vetter
2020-11-27 18:06     ` Imre Deak
2020-11-27 18:06       ` Imre Deak
2020-12-01  0:18       ` Chery, Nanley G
2020-12-01  0:18         ` Chery, Nanley G
2020-12-01 12:04         ` Imre Deak
2020-12-01 12:04           ` Imre Deak
2020-12-11  7:04           ` Chery, Nanley G [this message]
2020-12-11  7:04             ` Chery, Nanley G
2020-12-14 16:22             ` Imre Deak
2020-12-14 16:22               ` Imre Deak
2020-11-30 10:00     ` Jani Nikula
2020-11-30 10:00       ` Jani Nikula

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=ac393a9d1b774766a35c299915f0cca5@intel.com \
    --to=nanley.g.chery@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dhinakaran.pandiyan@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=ville.syrjala@linux.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.