[Mesa-dev] [PATCH 2/2] gallium: Add renderonly-based support for pl111+vc4.

[Eric Anholt]

Emil Velikov <emil.l.velikov at gmail.com> writes:

> Hi Eric,
>
> On 11 May 2017 at 00:06, Eric Anholt <eric at anholt.net> wrote:
>> This follows the model of imx (display) and etnaviv (render): pl111 is a
>> display-only device, so when asked to do GL for it, we see if we have a
>> vc4 renderer, make the vc4 screen, and have vc4 call back to pl111 to do
>> scanout allocations.
>>
>> The difference from etnaviv is that we share the same BO between vc4 and
>> pl111, rather than having a vc4 bo and a pl11 bo and copies between the
>> two.  The only mismatch between their requirements is that vc4 requires
>> 4-pixel (at 32bpp) stride alignment, while pl111 requires that stride
>> match width.  The kernel will reject any modesets to an incorrect stride,
>> so the 3D driver doesn't need to worry about that.
>> ---
> I'm not familiar with the hardware itself so I cannot comment on those.
> There's a couple of small notes within, but overall the patch looks good.
>
>>  .travis.yml                                        |  2 +-
>>  Makefile.am                                        |  2 +-
> Yes, thank you!
>
>
>> --- a/Android.mk
>> +++ b/Android.mk
>
>>  classic_drivers := i915 i965
>> -gallium_drivers := swrast freedreno i915g nouveau r300g r600g radeonsi vmwgfx vc4 virgl
>> +gallium_drivers := swrast freedreno i915g nouveau pl111 r300g r600g radeonsi vmwgfx vc4 virgl
>>
> The recent Android cleanups by RobH which will cause a clash. The gist
> is below, but feel free to skim through commit
> 3f097396a1642bb7033002d0bdd37e194afce06a.
>  - rework for the new gallium_drivers format
>  - add a couple of lines in src/gallium/drivers/pl111/Android.mk
> analogous to the vc4 - the "ifneq $HAVE_foo" hunk
>  - drop the guard in src/gallium/Android.mk
>
>
>> +++ b/src/gallium/winsys/pl111/drm/pl111_drm_winsys.c
>
>> -#include <unistd.h>
>>  #include <fcntl.h>
>> +#include <unistd.h>
>>
>> -#include "vc4_drm_public.h"
>> +#include "pl111_drm_public.h"
>> +#include "vc4/drm/vc4_drm_public.h"
>> +#include "xf86drm.h"
>>
>> -#include "vc4/vc4_screen.h"
>> +#include "pipe/p_screen.h"
>> +#include "renderonly/renderonly.h"
>
>> +#include "util/u_format.h"
> Seems unused.
>
>>
>> -struct pipe_screen *
>> -vc4_drm_screen_create(int fd)
>> +struct pipe_screen *pl111_drm_screen_create(int fd)
>>  {
>> -       return vc4_screen_create(fcntl(fd, F_DUPFD_CLOEXEC, 3));
>> +   struct renderonly ro = {
>> +      /* Passes the vc4-allocated BO through to the pl111 DRM device using
>> +       * PRIME buffer sharing.  The VC4 BO must be linear, which the SCANOUT
>> +       * flag on allocation will have ensured.
>> +       */
>> +      .create_for_resource = renderonly_create_gpu_import_for_resource,
>> +      .kms_fd = fd,
>> +      .gpu_fd = drmOpenWithType("vc4", NULL, DRM_NODE_RENDER),
> Please don't use the drmOpen* API. It's a legacy dragon with very
> subtle and inner workings.
> Using drmGetDevice[s] should provide any information that you need.
> Alternatively please let us know what's missing so we can address it.

One this platform, there are exactly two devices, one is vc4 and the
other is pl111.  drmOpenWithType is exactly the API we want, and if you
want something else, then you should probably replace its insides
instead of telling people to use a different API.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170517/cd502285/attachment.sig>