Re: [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 08 Apr 2026, Rob Clark <rob.clark@oss.qualcomm.com> wrote:
> On Wed, Apr 8, 2026 at 2:17 AM Jani Nikula <jani.nikula@intel.com> wrote:
>>
>> On Tue, 07 Apr 2026, Harshdeep Dhatt <harshdeepdhatt@gmail.com> wrote:
>> > The msm drm driver that runs on android platforms sets up its writeback
>> > connectors as DRM_MODE_CONNECTOR_VIRTUAL instead of DRM_MODE_CONNECTOR_WRITEBACK.
>> > This means when setting up the drm resources, these connectors get exposed
>> > to userspace even though userspace has not explicitly asked for writeback
>> > connectors. These connectors are by default in the connected state. When we
>> > try to configure one of the crtc's for this output (to get the primary plane),
>> > the msm driver bails because this connector (being a writeback connector)
>> > doesn't have an output framebuffer assigned to it. To avoid this failure, get the
>> > correct property id for setting up the output framebuffer id for writeback connectors
>> > as defined by the msm driver on android. Allocate and bind a dummy output fb for
>> > these connectors.
>>
>> Is this msm android driver in upstream linux kernel?
>
> I _think_ this is about the downstream android driver.  Which AFAIU
> relies enough on custom properties/etc that there may be a lot of
> "quirks" compared to an upstream drm/kms driver.

Right.

>> If not, why should we add this in upstream IGT?

So the question remains. And where do you draw the line with the
plethora of downstream vendor kernels and drivers?

BR,
Jani.

>>
>> BR,
>> Jani.
>>
>>
>> >
>> > Signed-off-by: Harshdeep Dhatt <harshdeepdhatt@gmail.com>
>> > ---
>> >  lib/drmtest.c |  5 +++++
>> >  lib/drmtest.h |  1 +
>> >  lib/igt_kms.c | 28 ++++++++++++++++++++++++++++
>> >  3 files changed, 34 insertions(+)
>> >
>> > diff --git a/lib/drmtest.c b/lib/drmtest.c
>> > index 4a788ea7a..2e6895618 100644
>> > --- a/lib/drmtest.c
>> > +++ b/lib/drmtest.c
>> > @@ -143,6 +143,11 @@ bool is_msm_device(int fd)
>> >       return __is_device(fd, "msm");
>> >  }
>> >
>> > +bool is_msm_android_device(int fd)
>> > +{
>> > +     return __is_device(fd, "msm_drm");
>> > +}
>> > +
>> >  bool is_nouveau_device(int fd)
>> >  {
>> >       /* Currently all nouveau-specific codepaths require libdrm */
>> > diff --git a/lib/drmtest.h b/lib/drmtest.h
>> > index 37874d729..c042a2b20 100644
>> > --- a/lib/drmtest.h
>> > +++ b/lib/drmtest.h
>> > @@ -147,6 +147,7 @@ bool is_amdgpu_device(int fd);
>> >  bool is_i915_device(int fd);
>> >  bool is_mtk_device(int fd);
>> >  bool is_msm_device(int fd);
>> > +bool is_msm_android_device(int fd);
>> >  bool is_nouveau_device(int fd);
>> >  bool is_vc4_device(int fd);
>> >  bool is_xe_device(int fd);
>> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> > index 04b1f3972..3948426bc 100644
>> > --- a/lib/igt_kms.c
>> > +++ b/lib/igt_kms.c
>> > @@ -950,6 +950,14 @@ igt_atomic_fill_connector_props(igt_display_t *display, igt_output_t *output,
>> >       for (i = 0; i < props->count_props; i++) {
>> >               drmModePropertyPtr prop =
>> >                       drmModeGetProperty(fd, props->props[i]);
>> > +             /*
>> > +              * msm android driver installs writeback fb id as a custom property which can be
>> > +              * identified by "FB_ID" string
>> > +              */
>> > +             if (is_msm_android_device(fd) && (!strcmp("FB_ID", prop->name))) {
>> > +                     output->props[IGT_CONNECTOR_WRITEBACK_FB_ID] = props->props[i];
>> > +                     continue;
>> > +             }
>> >
>> >               for (j = 0; j < num_connector_props; j++) {
>> >                       if (strcmp(prop->name, conn_prop_names[j]) != 0)
>> > @@ -1003,6 +1011,7 @@ static igt_plane_t *igt_get_assigned_primary(igt_output_t *output,
>> >       int drm_fd = output->display->drm_fd;
>> >       drmModeModeInfo *mode;
>> >       struct igt_fb fb;
>> > +     struct igt_fb out_fb = {0};
>> >       igt_plane_t *plane = NULL;
>> >       uint32_t crtc_id;
>> >       int i;
>> > @@ -1016,6 +1025,23 @@ static igt_plane_t *igt_get_assigned_primary(igt_output_t *output,
>> >
>> >       crtc_id = crtc->crtc_id;
>> >
>> > +     /*
>> > +      * msm android driver exposes writeback connectors as DRM_MODE_CONNECTOR_VIRTUAL and not
>> > +      * DRM_MODE_CONNECTOR_WRITEBACK. The driver will bail if the output fb is not specified for
>> > +      * this connector. Hence, allocate a dummy output fb here and assign it to the connector.
>> > +      */
>> > +     if (is_msm_android_device(drm_fd) && igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID)) {
>> > +             igt_require(igt_create_fb(drm_fd, mode->hdisplay, mode->vdisplay,
>> > +                     DRM_FORMAT_XRGB8888,
>> > +                     DRM_FORMAT_MOD_NONE,
>> > +                     &out_fb) == 0);
>> > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, out_fb.fb_id);
>> > +             igt_assert(drmModeConnectorSetProperty(drm_fd, output->id,
>> > +                     output->props[IGT_CONNECTOR_WRITEBACK_FB_ID],
>> > +                     output->values[IGT_CONNECTOR_WRITEBACK_FB_ID]) == 0);
>> > +     }
>> > +
>> > +
>> >       /*
>> >        * Do a legacy SETCRTC to start things off, so that we know that
>> >        * the kernel will pick the correct primary plane and attach it
>> > @@ -1039,6 +1065,8 @@ static igt_plane_t *igt_get_assigned_primary(igt_output_t *output,
>> >
>> >       /* Removing the FB will also shut down the display for us: */
>> >       igt_remove_fb(drm_fd, &fb);
>> > +     if (out_fb.fb_id != 0)
>> > +             igt_remove_fb(drm_fd, &out_fb);
>> >       igt_assert_f(plane, "Valid assigned primary plane for CRTC_ID %d not found.\n", crtc_id);
>> >
>> >       return plane;
>>
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel