All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Smith, Gary K" <gary.k.smith@intel.com>
To: Daniel Stone <daniel@fooishbar.org>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/i915: Render decompression support for Gen9 and above
Date: Tue, 22 Mar 2016 14:59:18 +0000	[thread overview]
Message-ID: <591D9F2F8137E144B6CDD649149EA82D1B4E5AEB@IRSMSX103.ger.corp.intel.com> (raw)
In-Reply-To: <CAPj87rPWiUim6B1pOOoLLMmVD_epUVEUnnvY1--PqVnpeo8T_A@mail.gmail.com>

I thought we had concluded previously that this change was acceptable, which is why I'm surprised that it's come up yet again.

The performance cost of creating two fb objects per buffer is irrelevant - it will be a one shot hit on buffer creation, and from that point forward it's just selecting which of the two fb's to use at any point in time. Given that I'm told that the memory cost kernel side per fb is small, the number isn't a big deal either. Hence, I'm not sure why you were expecting a performance analysis.

The two things I object to are:

1) Having to tell the kernel that there is no aux buffer on an allocation that actually has an aux buffer in order to indicate that at this point in time the buffer is uncompressed. This is completely non intuitive from a caller perspective - especially as it's called an aux buffer, not a compression buffer.

2) Having to use a fb object to manage dynamic state. The fb for a particular buffer will change over the course of a frame. Any debug for a frame at entry to the HWC will have a different fb to the debug at frame exit which makes it hard to track.

Thanks
Gary


-----Original Message-----
From: Daniel Stone [mailto:daniel@fooishbar.org] 
Sent: Tuesday, March 22, 2016 1:37 PM
To: Smith, Gary K <gary.k.smith@intel.com>
Cc: Kannan, Vandana <vandana.kannan@intel.com>; intel-gfx <intel-gfx@lists.freedesktop.org>; Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH 2/2] drm/i915: Render decompression support for Gen9 and above

On 22 March 2016 at 13:30, Daniel Stone <daniel@fooishbar.org> wrote:
> Exactly the same as the last time we discussed it

I should add that I understand your previous objection that creating framebuffers on the fly is not performant enough, and you object to the effort of managing 100 rather than 50 framebuffers upfront (though honestly, if you get to 50 framebuffers you're already having to do some clever setup/management anyway). But in the last thread, Daniel Vetter asked for some performance numbers to bear out your objection that framebuffer creation is too costly, leading to getting it fixed if this is in fact the case (since other userspace relies on it being fast), but this performance analysis never arrived.

I'd still be interested in seeing that.

Cheers,
Daniel
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-03-22 14:59 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-18 16:50 [PATCH 1/2] drm: Add aux plane verification in addFB2 Vandana Kannan
2016-03-18 16:50 ` [PATCH 2/2] drm/i915: Render decompression support for Gen9 and above Vandana Kannan
2016-03-18 17:05   ` Daniel Stone
2016-03-21 12:20     ` Smith, Gary K
2016-03-22 13:30       ` Daniel Stone
2016-03-22 13:36         ` Daniel Stone
2016-03-22 14:59           ` Smith, Gary K [this message]
2016-03-18 17:12   ` Ville Syrjälä
2016-04-25  5:00     ` Kannan, Vandana
2016-04-29 14:57       ` [PATCH v2 " Vandana Kannan
2016-04-29 14:45         ` Ville Syrjälä
2016-05-09  6:02           ` Kannan, Vandana
2016-05-13 14:04             ` [PATCH v3 " Vandana Kannan
2016-05-24 11:05               ` Kannan, Vandana
2016-03-18 16:54 ` [PATCH 1/2] drm: Add aux plane verification in addFB2 Ville Syrjälä
2016-03-21 10:58 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
2016-04-29 14:34 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm: Add aux plane verification in addFB2 (rev2) Patchwork
2016-05-13 13:42 ` ✗ Ro.CI.BAT: failure for series starting with [1/2] drm: Add aux plane verification in addFB2 (rev3) 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=591D9F2F8137E144B6CDD649149EA82D1B4E5AEB@IRSMSX103.ger.corp.intel.com \
    --to=gary.k.smith@intel.com \
    --cc=daniel@fooishbar.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /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.