[Mesa-dev] [PATCH 2/2] kmsro: Extend to include hx8357d.

[Eric Anholt]

Eric Engestrom <eric.engestrom at intel.com> writes:

> On Thursday, 2018-10-25 09:39:10 -0700, Eric Anholt wrote:
>> This allows vc4 to initialize on the Adafruit PiTFT 3.5" touchscreen with
>> the new tinydrm driver I just submitted.  If this series extending the
>> pl111/kmsro driver is accepted, then I'll extend kmsro with the other
>> tinydrm drivers as well.
>> ---
>>  src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c | 5 +++++
>>  src/gallium/drivers/kmsro/Android.mk                | 1 +
>>  src/gallium/drivers/kmsro/Automake.inc              | 1 +
>>  src/gallium/targets/dri/meson.build                 | 1 +
>>  src/gallium/targets/dri/target.c                    | 1 +
>>  5 files changed, 9 insertions(+)
>> 
>> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> index 230bafe5e159..73ddab0cbf02 100644
>> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> @@ -111,6 +111,11 @@ static const struct drm_driver_descriptor driver_descriptors[] = {
>>          .create_screen = pipe_kmsro_create_screen,
>>          .configuration = pipe_default_configuration_query,
>>      },
>> +    {
>> +       .driver_name = "hx8357d",
>> +        .create_screen = pipe_kmsro_create_screen,
>> +        .configuration = pipe_default_configuration_query,
>> +    },
>
> Is that really all it takes to add a new userspace tinydrm driver? Impressive!
>
> (nit: `.driver_name` missing one space of indentation)
>
> Code-wise, patch #1 is:
> Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>
>
> Not completely sure this is all that's needed for this patch, so only:
> Acked-by: Eric Engestrom <eric.engestrom at intel.com>
>
> That said, ... (scroll down)
>
>>      {
>>          .driver_name = "virtio_gpu",
>>          .create_screen = pipe_virgl_create_screen,
>> diff --git a/src/gallium/drivers/kmsro/Android.mk b/src/gallium/drivers/kmsro/Android.mk
>> index 8a851024dc88..f6a444e8865b 100644
>> --- a/src/gallium/drivers/kmsro/Android.mk
>> +++ b/src/gallium/drivers/kmsro/Android.mk
>> @@ -35,5 +35,6 @@ include $(BUILD_STATIC_LIBRARY)
>>  
>>  ifneq ($(HAVE_GALLIUM_KMSRO),)
>>  GALLIUM_TARGET_DRIVERS += pl111
>> +GALLIUM_TARGET_DRIVERS += hx8357d
>>  $(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_kmsro)
>>  endif
>> diff --git a/src/gallium/drivers/kmsro/Automake.inc b/src/gallium/drivers/kmsro/Automake.inc
>> index 66d125cb440a..d5961c907653 100644
>> --- a/src/gallium/drivers/kmsro/Automake.inc
>> +++ b/src/gallium/drivers/kmsro/Automake.inc
>> @@ -1,6 +1,7 @@
>>  if HAVE_GALLIUM_KMSRO
>>  
>>  TARGET_DRIVERS += pl111
>> +TARGET_DRIVERS += hx8357d
>>  TARGET_CPPFLAGS += -DGALLIUM_KMSRO
>>  TARGET_LIB_DEPS += \
>>      $(top_builddir)/src/gallium/winsys/kmsro/drm/libkmsrodrm.la \
>> diff --git a/src/gallium/targets/dri/meson.build b/src/gallium/targets/dri/meson.build
>> index c1cb616b4dad..bc63702498ba 100644
>> --- a/src/gallium/targets/dri/meson.build
>> +++ b/src/gallium/targets/dri/meson.build
>> @@ -63,6 +63,7 @@ libgallium_dri = shared_library(
>>  )
>>  
>>  foreach d : [[with_gallium_kmsro, 'pl111_dri.so'],
>> +             [with_gallium_kmsro, 'hx8357d_dri.so'],
>
> ... wouldn't we want the user-facing `gallium` option to have the
> `pl111,hx8357d` granularity?
>
> It would be more work, so as a follow-up patch, and they're just
> hardlinks of the same file anyway so they don't really take any disk
> space or compilation time, so I'm not sure it's worth it, but asking the
> question anyway :)

My thought was that since each of the tinydrm gallium drivers is the
same code and just one more loader entrypoint and drm_driver_descriptor
struct, everyone's better off if we don't list a million names in the
meson/autotools options and you just get them all as a group.

What I actually think I want is the loader to know that if you didn't
find a native driver and you've got a KMS node, it should try seeing if
kmsro_dri.so likes your KMS node.  However, that would require Xorg to
also know about this idea (when can we switch AIGLX over to EGL?).
-------------- 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/20181026/2bf78b0c/attachment.sig>