Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH 0/3] drm: omapdrm: Fix excessive GEM buffers DMM/CMA usage
To:     Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>, tomba@kernel.org,
        airlied@linux.ie, daniel@ffwll.ch
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        linux-omap@vger.kernel.org, merlijn@wizzup.org, tony@atomide.com
References: <1642587791-13222-1-git-send-email-ivo.g.dimitrov.75@gmail.com>
 <be4e1cd8-a994-303d-9424-14439ce1f7d4@ideasonboard.com>
From:   Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Message-ID: <47dbc690-bff2-8839-f01f-9287403a7562@gmail.com>
Date:   Thu, 17 Feb 2022 18:21:47 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Firefox/60.0 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <be4e1cd8-a994-303d-9424-14439ce1f7d4@ideasonboard.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Tomi,

On 17.02.22 г. 15:03 ч., Tomi Valkeinen wrote:
> Hi Ivaylo,
> 
> On 19/01/2022 12:23, Ivaylo Dimitrov wrote:
>> This patch series fixes excessive DMM or CMA usage of GEM buffers 
>> leading to
>> various runtime allocation failures. The series enables daily usage of 
>> devices
>> without exausting limited resources like CMA or DMM space if GPU 
>> rendering is
>> needed.
>>
>> The first patch doesn't bring any functional changes, it just moves some
>> TILER/DMM related code to a separate function, to simplify the review 
>> of the
>> next two patches.
>>
>> The second patch allows off-CPU rendering to non-scanout buffers. 
>> Without that
>> patch, it is basically impossible to use the driver allocated GEM 
>> buffers on
>> OMAP3 for anything else but a basic CPU rendered examples as if we 
>> want GPU
>> rendering, we must allocate buffers as scanout buffers, which are CMA 
>> allocated.
>> CMA soon gets fragmented and we start seeing allocation failures. Such 
>> failres
>> in Xorg cannot be handeled gracefully, so the system is basically 
>> unusable.
>>
>> Third patch fixes similar issue on OMAP4/5, where DMM/TILER spaces get
>> fragmented with time, leading to allocation failures.
> 
> I think this is just hacking around the problem. The problem is that 
> omapdrm is being used by some as a generic buffer allocator. Those users 

Well, the user of omap_bo interface I know is xf86-video-omap. Unless if 
by users you mean 'kernel users' which I know none.

I think that if 'we' are to teach xorg omap DDX (or any other user in 
that regard) to use GPU driver allocator for non-scanout buffers and 
omapdrm for scanout, it will become a mess. Not impossible though, just 
way more complicated than the $series. Also, why do omapdrm allow 
allocation of non-linear buffers and CPU (userspace) access to them, but 
refuses to export them to kernel drivers? Isn't that the whole point of 
DMABUF stuff? This is not consistent to me. The series fixes that 
inconsistency, nothing more.

> should be changed to use a their own allocator or a generic allocator. 

SGX driver/userspace has and uses its own allocator, however, I think 
there is more than that - what about TILER/VRFB? Do you say that SGX 
userspace shall be smart enough to requests TILER buffers from omapdrm 
when scanout buffer is requested and use its own allocator when not?

Actually I was thinking about something like that, and it is achievable 
now we have:

https://github.com/maemo-leste/sgx-ddk-um/blob/master/dbm/dbm.c (REed 
SGX 1.17 ddk gbm backend)

> And we could then drop the OMAP_BO_SCANOUT flag, as all buffers would be 
> scanout buffers.
> 

And what about OMAP_BO_TILED_XX stuff? To me this is even more of a 
hack, but it is what it is.

Do I get it correctly that you want to get rid of omap_bo_new/_tiled and 
have only dumb buffers available in omapdrm? TBH this would be great, 
however I still don't see how a TILER/VRFB buffer would be allocated, 
given that flags in drm_mode_create_dumb is not used anywhere in the 
kernel(AFAIK). Unless all scanout buffers are allocated through 
TILER/VRFB (which is a good idea IMO).

> Or do we have a regression in the driver? My understanding is that this 
> has never really worked.
> 

There are couple of patches in omapdrm that change around BO flags and 
their meaning so I think there is a regression, as the same 
userspace/DDX on linux 5.9 results in only 2 linear buffers being 
allocated, but as SGX driver has different version as well, I can't be 
100% sure without going through a lengthy assessment of SGX 
driver/omapdrm code and patches since 5.9. Which I am not going to do as 
I don't see what the benefit will be.

Please consider this patch series as a fix to an inconsistency, as it is 
merely that, it does not really bring any new functionality in terms of 
what is allocated.

Thanks and regards,
Ivo