public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android
@ 2026-04-07 20:29 Harshdeep Dhatt
  2026-04-08  9:17 ` Jani Nikula
  2026-04-09 17:50 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
  0 siblings, 2 replies; 8+ messages in thread
From: Harshdeep Dhatt @ 2026-04-07 20:29 UTC (permalink / raw)
  To: igt-dev; +Cc: rob.clark, Harshdeep Dhatt

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.

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;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android
  2026-04-07 20:29 [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android Harshdeep Dhatt
@ 2026-04-08  9:17 ` Jani Nikula
  2026-04-08 14:43   ` Rob Clark
  2026-04-09 17:50 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
  1 sibling, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2026-04-08  9:17 UTC (permalink / raw)
  To: Harshdeep Dhatt, igt-dev; +Cc: rob.clark, Harshdeep Dhatt

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?

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android
  2026-04-08  9:17 ` Jani Nikula
@ 2026-04-08 14:43   ` Rob Clark
  2026-04-08 15:29     ` Jani Nikula
  0 siblings, 1 reply; 8+ messages in thread
From: Rob Clark @ 2026-04-08 14:43 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Harshdeep Dhatt, igt-dev, Dmitry Baryshkov

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.

BR,
-R

> If not, why should we add this in upstream IGT?
>
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android
  2026-04-08 14:43   ` Rob Clark
@ 2026-04-08 15:29     ` Jani Nikula
       [not found]       ` <20260408162505.2131099-1-harshdeepdhatt@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2026-04-08 15:29 UTC (permalink / raw)
  To: rob.clark; +Cc: Harshdeep Dhatt, igt-dev, Dmitry Baryshkov

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android
       [not found]       ` <20260408162505.2131099-1-harshdeepdhatt@gmail.com>
@ 2026-04-08 17:21         ` Jani Nikula
  2026-04-08 17:46           ` Rob Clark
  2026-04-10 11:56           ` Kamil Konieczny
  0 siblings, 2 replies; 8+ messages in thread
From: Jani Nikula @ 2026-04-08 17:21 UTC (permalink / raw)
  To: Harshdeep Dhatt, Kamil Konieczny, Juha-Pekka Heikkila,
	Ashutosh Dixit, Simona Vetter
  Cc: rob.clark, igt-dev, lumag, Harshdeep Dhatt

On Wed, 08 Apr 2026, Harshdeep Dhatt <harshdeepdhatt@gmail.com> wrote:
> Thanks for the feedback.
>
> Yes, this quirk is specifically for the downstream MSM DRM driver used on
> Android devices. The upstream MSM driver does not need this, and the intent
> is not to modify or affect upstream MSM behavior in any way.
>
> The goal here is to allow IGT to run on Qualcomm-based Android platforms,
> which currently expose writeback connectors differently (as VIRTUAL
> connectors with a custom "FB_ID" property). Without assigning a writeback
> FB, the driver rejects the atomic commit, causing several IGT tests to fail
> early.
>
> All of the logic in this patch is fully isolated behind detection of the
> downstream Android MSM driver (`"msm_drm"`). Upstream MSM continues to use
> the existing code paths unchanged.
>
> IGT’s documentation states that it aims to be a universal test suite for
> DRM/KMS drivers on Linux *and similar platforms*. Supporting the downstream
> MSM driver helps extend IGT coverage to a widely deployed class of devices
> without impacting upstream users.
>
> If it would be preferable, I can restructure this into a dedicated
> “msm-android quirk” helper or move the detection/handling into a separate
> file so that the core paths remain untouched. 
>
> This keeps all non-upstream behavior contained in one place, avoids
> polluting the main code paths, and provides a maintainable structure for
> any future quirks that may be needed for other platforms. Upstream MSM
> remains unaffected, and the quirk only activates when running on the
> downstream Android MSM driver.
>
> I’m happy to adjust the approach based on your guidance.

Cc'd some more folks.

As a contributor, I'm not very fond of the idea of tiptoeing around code
in IGT that caters for some downstream vendor kernels. I'm not going to
go hunting for those sources. Hardly anyone will.

I'm not an IGT maintainer. I'm not making decisions on what is, and is
not, acceptable new maintenance burden to take on.

But the maintainers do have a decision to make. The options are to
support either 0 or N downstream kernels. If you take one, you can't
reasonably refuse anything after that. Even if the patch at hand is
benign, the precedent it sets is emphatically not.


BR,
Jani.


-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android
  2026-04-08 17:21         ` Jani Nikula
@ 2026-04-08 17:46           ` Rob Clark
  2026-04-10 11:56           ` Kamil Konieczny
  1 sibling, 0 replies; 8+ messages in thread
From: Rob Clark @ 2026-04-08 17:46 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Harshdeep Dhatt, Kamil Konieczny, Juha-Pekka Heikkila,
	Ashutosh Dixit, Simona Vetter, igt-dev, lumag

On Wed, Apr 8, 2026 at 10:21 AM Jani Nikula <jani.nikula@intel.com> wrote:
>
> On Wed, 08 Apr 2026, Harshdeep Dhatt <harshdeepdhatt@gmail.com> wrote:
> > Thanks for the feedback.
> >
> > Yes, this quirk is specifically for the downstream MSM DRM driver used on
> > Android devices. The upstream MSM driver does not need this, and the intent
> > is not to modify or affect upstream MSM behavior in any way.
> >
> > The goal here is to allow IGT to run on Qualcomm-based Android platforms,
> > which currently expose writeback connectors differently (as VIRTUAL
> > connectors with a custom "FB_ID" property). Without assigning a writeback
> > FB, the driver rejects the atomic commit, causing several IGT tests to fail
> > early.
> >
> > All of the logic in this patch is fully isolated behind detection of the
> > downstream Android MSM driver (`"msm_drm"`). Upstream MSM continues to use
> > the existing code paths unchanged.
> >
> > IGT’s documentation states that it aims to be a universal test suite for
> > DRM/KMS drivers on Linux *and similar platforms*. Supporting the downstream
> > MSM driver helps extend IGT coverage to a widely deployed class of devices
> > without impacting upstream users.

Without weighing in on whether igt should support downstream drm
drivers (I don't have skin in that game), I will point out that "and
similar platforms" was probably intended to refer to things like the
*BSDs which port upstream drm drivers to their own OS, rather than
downstream drm drivers.

BR,
-R

> > If it would be preferable, I can restructure this into a dedicated
> > “msm-android quirk” helper or move the detection/handling into a separate
> > file so that the core paths remain untouched.
> >
> > This keeps all non-upstream behavior contained in one place, avoids
> > polluting the main code paths, and provides a maintainable structure for
> > any future quirks that may be needed for other platforms. Upstream MSM
> > remains unaffected, and the quirk only activates when running on the
> > downstream Android MSM driver.
> >
> > I’m happy to adjust the approach based on your guidance.
>
> Cc'd some more folks.
>
> As a contributor, I'm not very fond of the idea of tiptoeing around code
> in IGT that caters for some downstream vendor kernels. I'm not going to
> go hunting for those sources. Hardly anyone will.
>
> I'm not an IGT maintainer. I'm not making decisions on what is, and is
> not, acceptable new maintenance burden to take on.
>
> But the maintainers do have a decision to make. The options are to
> support either 0 or N downstream kernels. If you take one, you can't
> reasonably refuse anything after that. Even if the patch at hand is
> benign, the precedent it sets is emphatically not.
>
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✗ LGCI.VerificationFailed: failure for lib/igt_kms: Identify writeback connectors in msm driver on android
  2026-04-07 20:29 [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android Harshdeep Dhatt
  2026-04-08  9:17 ` Jani Nikula
@ 2026-04-09 17:50 ` Patchwork
  1 sibling, 0 replies; 8+ messages in thread
From: Patchwork @ 2026-04-09 17:50 UTC (permalink / raw)
  To: Harshdeep Dhatt; +Cc: igt-dev

== Series Details ==

Series: lib/igt_kms: Identify writeback connectors in msm driver on android
URL   : https://patchwork.freedesktop.org/series/164501/
State : failure

== Summary ==

Address 'harshdeepdhatt@gmail.com' is not on the allowlist, which prevents CI from being triggered for this patch.
If you want Intel GFX CI to accept this address, please contact the script maintainers at i915-ci-infra@lists.freedesktop.org.
Exception occurred during validation, bailing out!



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android
  2026-04-08 17:21         ` Jani Nikula
  2026-04-08 17:46           ` Rob Clark
@ 2026-04-10 11:56           ` Kamil Konieczny
  1 sibling, 0 replies; 8+ messages in thread
From: Kamil Konieczny @ 2026-04-10 11:56 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Harshdeep Dhatt, Juha-Pekka Heikkila, Ashutosh Dixit,
	Simona Vetter, rob.clark, igt-dev, lumag, Karthik B S,
	Swati Sharma

Hi Jani,
On 2026-04-08 at 20:21:44 +0300, Jani Nikula wrote:
> On Wed, 08 Apr 2026, Harshdeep Dhatt <harshdeepdhatt@gmail.com> wrote:
> > Thanks for the feedback.
> >
> > Yes, this quirk is specifically for the downstream MSM DRM driver used on
> > Android devices. The upstream MSM driver does not need this, and the intent
> > is not to modify or affect upstream MSM behavior in any way.
> >
> > The goal here is to allow IGT to run on Qualcomm-based Android platforms,
> > which currently expose writeback connectors differently (as VIRTUAL
> > connectors with a custom "FB_ID" property). Without assigning a writeback
> > FB, the driver rejects the atomic commit, causing several IGT tests to fail
> > early.

Why is it different from mainstream Linux?
imho better way would be to bring that changes to upstream MSM driver
or just fork IGT source for your testing purposes.

> >
> > All of the logic in this patch is fully isolated behind detection of the
> > downstream Android MSM driver (`"msm_drm"`). Upstream MSM continues to use
> > the existing code paths unchanged.
> >
> > IGT’s documentation states that it aims to be a universal test suite for
> > DRM/KMS drivers on Linux *and similar platforms*. Supporting the downstream
> > MSM driver helps extend IGT coverage to a widely deployed class of devices
> > without impacting upstream users.
> >
> > If it would be preferable, I can restructure this into a dedicated
> > “msm-android quirk” helper or move the detection/handling into a separate
> > file so that the core paths remain untouched. 
> >
> > This keeps all non-upstream behavior contained in one place, avoids
> > polluting the main code paths, and provides a maintainable structure for
> > any future quirks that may be needed for other platforms. Upstream MSM
> > remains unaffected, and the quirk only activates when running on the
> > downstream Android MSM driver.
> >
> > I’m happy to adjust the approach based on your guidance.
> 
> Cc'd some more folks.
> 
> As a contributor, I'm not very fond of the idea of tiptoeing around code
> in IGT that caters for some downstream vendor kernels. I'm not going to
> go hunting for those sources. Hardly anyone will.
> 
> I'm not an IGT maintainer. I'm not making decisions on what is, and is
> not, acceptable new maintenance burden to take on.
> 
> But the maintainers do have a decision to make. The options are to
> support either 0 or N downstream kernels. If you take one, you can't
> reasonably refuse anything after that. Even if the patch at hand is
> benign, the precedent it sets is emphatically not.
> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

Added few more KMS devs on Cc
Cc: Karthik B S <karthik.b.s@intel.com>
Cc: Swati Sharma <swati2.sharma@intel.com>

Regards,
Kamil

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-04-10 11:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07 20:29 [PATCH i-g-t] lib/igt_kms: Identify writeback connectors in msm driver on android Harshdeep Dhatt
2026-04-08  9:17 ` Jani Nikula
2026-04-08 14:43   ` Rob Clark
2026-04-08 15:29     ` Jani Nikula
     [not found]       ` <20260408162505.2131099-1-harshdeepdhatt@gmail.com>
2026-04-08 17:21         ` Jani Nikula
2026-04-08 17:46           ` Rob Clark
2026-04-10 11:56           ` Kamil Konieczny
2026-04-09 17:50 ` ✗ LGCI.VerificationFailed: failure for " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox