[Adding Rob & Thierry]

On 09/09/2015 09:36 AM, Smith, Gary K wrote:
> I don't understand why this is an issue. Surely the fb is to describe static 
> state about the buffer, not dynamic state. The fb should be created with the 
> compressed modifier. The compressed property is just a hint to the kernel 
> that the buffer has been completely resolved, hence currently it can be 
> treated as an uncompressed fb (and the aux buffer can be ignored). This is 
> dynamic state that may well change very regularly over the lifetime of the 
> buffer.
> 
> It's still a compressed fb, it contains a aux buffer and had to be created 
> with the compressed fb modifier. However, once the userspace has fully 
> resolved the buffer, the aux buffer can be ignored and the compressed fb can 
> be used in any situation where an uncompressed fb would normally be required. 
> This is dynamic state that may well change very regularly over the lifetime 
> of the buffer.
> 
> I could allocate two fbs always, and use the appropriate one. We already do 
> this in order to indicate whether a RGBA buffer currently needs to be 
> considered as opaque (RGBX) or blended. Experience has shown that it makes it 
> very complex to debug when the fb keeps on changing its value. However, 
> because we now have 4 different states (Blended/Opaque and Compressed or 
> Resolved), we will now end up with up to 4 fbs per buffer.
> 
> We aren't just talking about a few fbs here, we already see more than 100 fbs 
> active during complex situations. Potentially doubling this number is surely 
> a significant increase in memory usage, both from the management side in 
> userspace and the kernel side.
> 
> Thanks
> Gary
> 
> 
> 
> -----Original Message-----
> From: Jesse Barnes [mailto:[email protected]] 
> Sent: Wednesday, September 9, 2015 4:25 PM
> To: Daniel Vetter
> Cc: Kannan, Vandana; [email protected]; Smith, Gary K
> Subject: Re: [Intel-gfx] [RFC] drm/i915: Render decompression support for 
> Gen9 and above
> 
> On 09/09/2015 08:23 AM, Daniel Vetter wrote:
>> On Tue, Sep 08, 2015 at 03:07:40PM -0700, Jesse Barnes wrote:
>>> On 09/07/2015 09:35 AM, Daniel Vetter wrote:
>>>> On Sat, Sep 05, 2015 at 01:12:50AM +0530, Vandana Kannan wrote:
>>>>> This patch includes enabling render decompression after checking 
>>>>> all the requirements (format, tiling, rotation etc.). Along with 
>>>>> this, the WAs mentioned in BSpec Workaround page have been implemented.
>>>>>
>>>>> This patch has been implemented on top of Nabendu/Chandra's NV12 patches.
>>>>>
>>>>> TODO:
>>>>> 1. Disable stereo 3D when render decomp is enabled (bit 7:6) 2. 
>>>>> Render decompression must not be used in VTd pass-through mode 3. 
>>>>> Program hashing select CHICKEN_MISC1 bit 15 4. For Gen10, add 
>>>>> support for RGB 1010102
>>>>>
>>>>> Signed-off-by: Vandana Kannan <[email protected]>
>>>>> ---
>>>>>  drivers/gpu/drm/drm_atomic.c         |   4 +
>>>>>  drivers/gpu/drm/drm_crtc.c           |  16 ++++
>>>>>  drivers/gpu/drm/i915/intel_display.c | 174 
>>>>> +++++++++++++++++++++++++++++++++++
>>>>>  drivers/gpu/drm/i915/intel_drv.h     |   7 ++
>>>>>  drivers/gpu/drm/i915/intel_sprite.c  |  35 +++++++
>>>>>  include/drm/drm_crtc.h               |  11 +++
>>>>>  6 files changed, 247 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_atomic.c 
>>>>> b/drivers/gpu/drm/drm_atomic.c index 940f80b..d9004e8 100644
>>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>>> @@ -607,6 +607,8 @@ int drm_atomic_plane_set_property(struct drm_plane 
>>>>> *plane,
>>>>>           state->src_h = val;
>>>>>   } else if (property == config->rotation_property) {
>>>>>           state->rotation = val;
>>>>> + } else if (property == config->compression_property) {
>>>>> +         state->compression = val;
>>>>
>>>> Please use a framebuffer modifier instead. Also this needs userspace.
>>>
>>> I thought we already agreed, based on feedback from the userspace 
>>> guys, that a property was easier to use and therefore the way to go?
>>
>> Blob hwc want a property because they're afraid of the overhead of 
>> creating an additional drm fb object. Until I see data that that 
>> overhead is real I see no reason at all to have something else than 
>> what the community consensus for these features from 1 year ago at xdc 
>> bordeaux.
>>
>> If someone disagrees please convince Rob Clark and Thierry Redding 
>> (and whomever else took part in that discussion) that we need to 
>> change this, I personally don't see the value in this particular bikeshed.
> 
> I don't think it was overhead, just convenience and reasoning about how the 
> feature is used.  Cc'ing Gary for more background.
> 
> Jesse
> 
> ---------------------------------------------------------------------
> 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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to