* [RESEND PATCH libdrm v3 0/2] Add Writeback Support for Modetest
@ 2022-09-01 23:09 Jessica Zhang
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 1/2] tests/modetest: Allocate drmModeAtomicReq before setting properties Jessica Zhang
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 2/2] tests/modetest: Add support for writeback connector Jessica Zhang
0 siblings, 2 replies; 6+ messages in thread
From: Jessica Zhang @ 2022-09-01 23:09 UTC (permalink / raw)
To: dri-devel; +Cc: quic_abhinavk, hoegsberg, dmitry.baryshkov, Jessica Zhang
Resending to correct typo in email list and adding "Signed-off-by" to
Patch [1/2] ("tests/modetest: Allocate drmModeAtomicReq before setting
properties"
---
Add writeback support to modetest with the below options:
- Passing in -c will now also show the writeback connector
- Test a built-in mode on writeback connector
- Test a custom mode from user input on writeback connector
Usage: "./modetest -M msm -x <connector_id>:<mode_parameters>
-a -P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24."
Refer to --help for exact syntax
- Dump the writeback output buffer to bitstream
Usage: "./modetest -M msm -s <connector_id>:<widthxheight>
-a -o <filepath>
-P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24"
This currently supports a singular writeback connector.
This patch also fixes a bug for running modetest with the atomic flag.
Changes made in V2:
- Added helper method that checks if user pipe has writeback connector
- Added error message for dump flag if no writeback connector is found
- Polls on the writeback fence fd until writeback is complete
Changes made in V3:
- Resolved compiler warnings
- Defined ETIME to ETIMEDOUT in cases where ETIME is undefined
Rohith Iyer (2):
tests/modetest: Allocate drmModeAtomicReq before setting properties
tests/modetest: Add support for writeback connector
tests/modetest/buffers.c | 19 ++++
tests/modetest/buffers.h | 1 +
tests/modetest/modetest.c | 187 ++++++++++++++++++++++++++++++++++----
3 files changed, 187 insertions(+), 20 deletions(-)
--
2.31.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RESEND PATCH libdrm v3 1/2] tests/modetest: Allocate drmModeAtomicReq before setting properties
2022-09-01 23:09 [RESEND PATCH libdrm v3 0/2] Add Writeback Support for Modetest Jessica Zhang
@ 2022-09-01 23:09 ` Jessica Zhang
2022-09-02 6:10 ` Dmitry Baryshkov
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 2/2] tests/modetest: Add support for writeback connector Jessica Zhang
1 sibling, 1 reply; 6+ messages in thread
From: Jessica Zhang @ 2022-09-01 23:09 UTC (permalink / raw)
To: dri-devel
Cc: Rohith Iyer, quic_abhinavk, hoegsberg, dmitry.baryshkov, Jessica Zhang
From: Rohith Iyer <quic_rohiiyer@quicinc.com>
Fix null pointer deference caused by drmModeAtomicReq being
allocated before set_property was called when modetest was run
with the atomic flag.
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Rohith Iyer <quic_rohiiyer@quicinc.com>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
tests/modetest/modetest.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 42e2d1f4..2c31c4fc 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -2186,11 +2186,13 @@ int main(int argc, char **argv)
dump_resource(&dev, planes);
dump_resource(&dev, framebuffers);
+ if (dev.use_atomic)
+ dev.req = drmModeAtomicAlloc();
+
for (i = 0; i < prop_count; ++i)
set_property(&dev, &prop_args[i]);
if (dev.use_atomic) {
- dev.req = drmModeAtomicAlloc();
if (set_preferred || (count && plane_count)) {
uint64_t cap = 0;
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RESEND PATCH libdrm v3 2/2] tests/modetest: Add support for writeback connector
2022-09-01 23:09 [RESEND PATCH libdrm v3 0/2] Add Writeback Support for Modetest Jessica Zhang
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 1/2] tests/modetest: Allocate drmModeAtomicReq before setting properties Jessica Zhang
@ 2022-09-01 23:09 ` Jessica Zhang
2022-09-02 6:40 ` Dmitry Baryshkov
1 sibling, 1 reply; 6+ messages in thread
From: Jessica Zhang @ 2022-09-01 23:09 UTC (permalink / raw)
To: dri-devel
Cc: Rohith Iyer, quic_abhinavk, hoegsberg, dmitry.baryshkov, Jessica Zhang
From: Rohith Iyer <quic_rohiiyer@quicinc.com>
Add writeback support to modetest with the below options:
- Passing in -c will now also show the writeback connector
- Test a built-in mode on writeback connector
- Test a custom mode from user input on writeback connector
Usage: "./modetest -M msm -x <connector_id>:<mode_parameters>
-a -P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24."
Refer to --help for exact syntax
- Dump the writeback output buffer to bitstream
Usage: "./modetest -M msm -s <connector_id>:<widthxheight>
-a -o <filepath>
-P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24"
This currently supports a singular writeback connector.
Changes made in V2:
- Added helper method that checks if user pipe has writeback connector
- Added error message for dump flag if no writeback connector is found
- Polls on the writeback fence fd until writeback is complete
Changes made in V3:
- Resolved compiler warnings
- Defined ETIME to ETIMEDOUT in cases where ETIME is undefined
Co-developed-by: Rohith Iyer <quic_rohiiyer@quicinc.com>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
tests/modetest/buffers.c | 19 ++++
tests/modetest/buffers.h | 1 +
tests/modetest/modetest.c | 183 ++++++++++++++++++++++++++++++++++----
3 files changed, 184 insertions(+), 19 deletions(-)
diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 8a8d9e01..279d7b28 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -353,3 +353,22 @@ void bo_destroy(struct bo *bo)
free(bo);
}
+
+void bo_dump(struct bo *bo, const char *filename)
+{
+ FILE *fp;
+
+ if (!bo || !filename)
+ return;
+
+ fp = fopen(filename, "wb");
+ if (fp) {
+ void *addr;
+
+ bo_map(bo, &addr);
+ printf("Dumping buffer %p to file %s.\n", bo->ptr, filename);
+ fwrite(bo->ptr, 1, bo->size, fp);
+ bo_unmap(bo);
+ fclose(fp);
+ }
+}
diff --git a/tests/modetest/buffers.h b/tests/modetest/buffers.h
index 7f95396b..cbd54e9e 100644
--- a/tests/modetest/buffers.h
+++ b/tests/modetest/buffers.h
@@ -36,5 +36,6 @@ struct bo *bo_create(int fd, unsigned int format,
unsigned int handles[4], unsigned int pitches[4],
unsigned int offsets[4], enum util_fill_pattern pattern);
void bo_destroy(struct bo *bo);
+void bo_dump(struct bo *bo, const char *filename);
#endif
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 2c31c4fc..8073d143 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -68,8 +68,13 @@
#include "buffers.h"
#include "cursor.h"
+#ifndef ETIME
+#define ETIME ETIMEDOUT
+#endif
+
static enum util_fill_pattern primary_fill = UTIL_PATTERN_SMPTE;
static enum util_fill_pattern secondary_fill = UTIL_PATTERN_TILES;
+static enum util_fill_pattern plain_fill = UTIL_PATTERN_PLAIN;
struct crtc {
drmModeCrtc *crtc;
@@ -128,6 +133,7 @@ struct device {
int use_atomic;
drmModeAtomicReq *req;
+ int32_t writeback_fence_fd;
};
static inline int64_t U642I64(uint64_t val)
@@ -135,6 +141,11 @@ static inline int64_t U642I64(uint64_t val)
return (int64_t)*((int64_t *)&val);
}
+static inline uint64_t to_user_pointer(const void *ptr)
+{
+ return (uintptr_t)ptr;
+}
+
static float mode_vrefresh(drmModeModeInfo *mode)
{
return mode->clock * 1000.00
@@ -811,6 +822,10 @@ struct pipe_arg {
struct crtc *crtc;
unsigned int fb_id[2], current_fb_id;
struct timeval start;
+ unsigned int out_fb_id;
+ struct bo *out_bo;
+ bool custom;
+ bool dump;
int swap_count;
};
@@ -917,27 +932,43 @@ static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe)
return &dev->resources->crtcs[crtc_idx - 1];
}
+static int parse_mode_string(char *mode_string, drmModeModeInfo *user_mode)
+{
+ return sscanf(mode_string, "%u,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%u,%s",
+ &user_mode->clock, &user_mode->hdisplay, &user_mode->hsync_start,
+ &user_mode->hsync_end, &user_mode->htotal, &user_mode->hskew,
+ &user_mode->vdisplay, &user_mode->vsync_start, &user_mode->vsync_end,
+ &user_mode->vtotal, &user_mode->vscan, &user_mode->vrefresh,
+ user_mode->name);
+}
+
static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
{
drmModeModeInfo *mode = NULL;
int i;
+ static drmModeModeInfo user_mode;
- pipe->mode = NULL;
-
- for (i = 0; i < (int)pipe->num_cons; i++) {
- mode = connector_find_mode(dev, pipe->con_ids[i],
- pipe->mode_str, pipe->vrefresh);
- if (mode == NULL) {
- if (pipe->vrefresh)
- fprintf(stderr,
- "failed to find mode "
- "\"%s-%.2fHz\" for connector %s\n",
- pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
- else
- fprintf(stderr,
- "failed to find mode \"%s\" for connector %s\n",
- pipe->mode_str, pipe->cons[i]);
+ if (pipe->custom) {
+ if (parse_mode_string(pipe->mode_str, &user_mode) == 13)
+ mode = &user_mode;
+ else
return -EINVAL;
+ } else {
+ for (i = 0; i < (int)pipe->num_cons; i++) {
+ mode = connector_find_mode(dev, pipe->con_ids[i],
+ pipe->mode_str, pipe->vrefresh);
+ if (mode == NULL) {
+ if (pipe->vrefresh)
+ fprintf(stderr,
+ "failed to find mode "
+ "\"%s-%.2fHz\" for connector %s\n",
+ pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
+ else
+ fprintf(stderr,
+ "failed to find mode \"%s\" for connector %s\n",
+ pipe->mode_str, pipe->cons[i]);
+ return -EINVAL;
+ }
}
}
@@ -1441,6 +1472,24 @@ static int pipe_resolve_connectors(struct device *dev, struct pipe_arg *pipe)
return 0;
}
+static bool pipe_has_writeback_connector(struct device *dev, struct pipe_arg *pipes,
+ unsigned int count)
+{
+ drmModeConnector *connector;
+ unsigned int i, j;
+
+ for (j = 0; j < count; j++) {
+ struct pipe_arg *pipe = &pipes[j];
+
+ for (i = 0; i < pipe->num_cons; i++) {
+ connector = get_connector_by_id(dev, pipe->con_ids[i]);
+ if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
+ return true;
+ }
+ }
+ return false;
+}
+
static int pipe_attempt_connector(struct device *dev, drmModeConnector *con,
struct pipe_arg *pipe)
{
@@ -1662,6 +1711,70 @@ static void set_mode(struct device *dev, struct pipe_arg *pipes, unsigned int co
}
}
+static void writeback_config(struct device *dev, struct pipe_arg *pipes, unsigned int count)
+{
+ drmModeConnector *connector;
+ unsigned int i, j;
+
+ for (j = 0; j < count; j++) {
+ struct pipe_arg *pipe = &pipes[j];
+
+ for (i = 0; i < pipe->num_cons; i++) {
+ connector = get_connector_by_id(dev, pipe->con_ids[i]);
+ if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) {
+ bo_fb_create(dev->fd, pipes[j].fourcc, pipe->mode->hdisplay,
+ pipe->mode->vdisplay, plain_fill, &pipe->out_bo,
+ &pipe->out_fb_id);
+ add_property(dev, pipe->con_ids[i], "WRITEBACK_FB_ID",
+ pipe->out_fb_id);
+ add_property(dev, pipe->con_ids[i], "WRITEBACK_OUT_FENCE_PTR",
+ to_user_pointer(&dev->writeback_fence_fd));
+ }
+ }
+ }
+}
+
+static int poll_writeback_fence(int fd, int timeout)
+{
+ struct pollfd fds = { fd, POLLIN };
+ int ret;
+
+ do {
+ ret = poll(&fds, 1, timeout);
+ if (ret > 0) {
+ if (fds.revents & (POLLERR | POLLNVAL))
+ return -EINVAL;
+
+ return 0;
+ } else if (ret == 0) {
+ return -ETIME;
+ } else {
+ ret = -errno;
+ if (ret == -EINTR || ret == -EAGAIN)
+ continue;
+ return ret;
+ }
+ } while (1);
+
+}
+
+static void dump_output_fb(struct device *dev, struct pipe_arg *pipes, char *dump_path,
+ unsigned int count)
+{
+ drmModeConnector *connector;
+ unsigned int i, j;
+
+ for (j = 0; j < count; j++) {
+ struct pipe_arg *pipe = &pipes[j];
+
+ for (i = 0; i < pipe->num_cons; i++) {
+ connector = get_connector_by_id(dev, pipe->con_ids[i]);
+ if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
+ bo_dump(pipe->out_bo, dump_path);
+ }
+ }
+}
+
static void atomic_clear_mode(struct device *dev, struct pipe_arg *pipes, unsigned int count)
{
unsigned int i;
@@ -1990,7 +2103,7 @@ static void parse_fill_patterns(char *arg)
static void usage(char *name)
{
- fprintf(stderr, "usage: %s [-acDdefMPpsCvrw]\n", name);
+ fprintf(stderr, "usage: %s [-acDdefMPpsCvrwx]\n", name);
fprintf(stderr, "\n Query options:\n\n");
fprintf(stderr, "\t-c\tlist connectors\n");
@@ -2000,7 +2113,12 @@ static void usage(char *name)
fprintf(stderr, "\n Test options:\n\n");
fprintf(stderr, "\t-P <plane_id>@<crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
- fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:[#<mode index>]<mode>[-<vrefresh>][@<format>]\tset a mode\n");
+ fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:[#<mode index>]");
+ fprintf(stderr, "<mode>[-<vrefresh>][@<format>]\tset a built-in mode\n");
+ fprintf(stderr, "\t-x <connector_id>[@<crtc_id>]:<clock,hdisplay,hsyncstart,hsyncend,");
+ fprintf(stderr, "htotal,hskew,vdisplay,vsyncstart,vsyncend,vtotal,vscan,");
+ fprintf(stderr, "vrefresh,name>\tset a custom mode\n");
+ fprintf(stderr, "\t-o <desired file path> \t Dump writeback output buffer to file\n");
fprintf(stderr, "\t-C\ttest hw cursor\n");
fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
fprintf(stderr, "\t-r\tset the preferred mode for all connectors\n");
@@ -2017,7 +2135,7 @@ static void usage(char *name)
exit(0);
}
-static char optstr[] = "acdD:efF:M:P:ps:Cvrw:";
+static const char optstr[] = "acdD:efF:M:o:P:ps:Cvrw:x:";
int main(int argc, char **argv)
{
@@ -2040,6 +2158,7 @@ int main(int argc, char **argv)
struct property_arg *prop_args = NULL;
unsigned int args = 0;
int ret;
+ char *dump_path;
memset(&dev, 0, sizeof dev);
@@ -2078,6 +2197,10 @@ int main(int argc, char **argv)
/* Preserve the default behaviour of dumping all information. */
args--;
break;
+ case 'o':
+ pipe_args->dump = true;
+ dump_path = optarg;
+ break;
case 'P':
plane_args = realloc(plane_args,
(plane_count + 1) * sizeof *plane_args);
@@ -2096,6 +2219,7 @@ int main(int argc, char **argv)
crtcs = 1;
planes = 1;
break;
+ case 'x':
case 's':
pipe_args = realloc(pipe_args,
(count + 1) * sizeof *pipe_args);
@@ -2107,7 +2231,7 @@ int main(int argc, char **argv)
if (parse_connector(&pipe_args[count], optarg) < 0)
usage(argv[0]);
-
+ pipe_args->custom = (c == 'x');
count++;
break;
case 'C':
@@ -2163,6 +2287,7 @@ int main(int argc, char **argv)
if (use_atomic) {
ret = drmSetClientCap(dev.fd, DRM_CLIENT_CAP_ATOMIC, 1);
+ drmSetClientCap(dev.fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
if (ret) {
fprintf(stderr, "no atomic modesetting support: %s\n", strerror(errno));
drmClose(dev.fd);
@@ -2206,6 +2331,12 @@ int main(int argc, char **argv)
if (set_preferred || count)
set_mode(&dev, pipe_args, count);
+ if (pipe_has_writeback_connector(&dev, pipe_args, count))
+ writeback_config(&dev, pipe_args, count);
+ else if (pipe_args->dump)
+ fprintf(stderr,
+ "No writeback connector found - dump will be skipped.\n");
+
if (plane_count)
atomic_set_planes(&dev, plane_args, plane_count, false);
@@ -2215,6 +2346,20 @@ int main(int argc, char **argv)
return 1;
}
+ /*
+ * Since only writeback connectors have an output fb, this should only be
+ * called for writeback.
+ */
+ if (pipe_has_writeback_connector(&dev, pipe_args, count)) {
+ ret = poll_writeback_fence(dev.writeback_fence_fd, 1000);
+ if (ret)
+ fprintf(stderr,
+ "Poll for writeback error: %d. Skipping Dump.\n",
+ ret);
+ else if (pipe_args->dump)
+ dump_output_fb(&dev, pipe_args, dump_path, count);
+ }
+
if (test_vsync)
atomic_test_page_flip(&dev, pipe_args, plane_args, plane_count);
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH libdrm v3 1/2] tests/modetest: Allocate drmModeAtomicReq before setting properties
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 1/2] tests/modetest: Allocate drmModeAtomicReq before setting properties Jessica Zhang
@ 2022-09-02 6:10 ` Dmitry Baryshkov
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2022-09-02 6:10 UTC (permalink / raw)
To: Jessica Zhang, dri-devel; +Cc: hoegsberg, Rohith Iyer, quic_abhinavk
On 2 September 2022 02:09:23 GMT+03:00, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>From: Rohith Iyer <quic_rohiiyer@quicinc.com>
>
>Fix null pointer deference caused by drmModeAtomicReq being
>allocated before set_property was called when modetest was run
>with the atomic flag.
... being allocated _after_ ...
Other than that:
Reviewed-by: Dmitry Baryshkov<dmitry.baryshkov@linaro.org>
>
>Reviewed-by: Rob Clark <robdclark@gmail.com>
>Signed-off-by: Rohith Iyer <quic_rohiiyer@quicinc.com>
>Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>---
> tests/modetest/modetest.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
>index 42e2d1f4..2c31c4fc 100644
>--- a/tests/modetest/modetest.c
>+++ b/tests/modetest/modetest.c
>@@ -2186,11 +2186,13 @@ int main(int argc, char **argv)
> dump_resource(&dev, planes);
> dump_resource(&dev, framebuffers);
>
>+ if (dev.use_atomic)
>+ dev.req = drmModeAtomicAlloc();
>+
> for (i = 0; i < prop_count; ++i)
> set_property(&dev, &prop_args[i]);
>
> if (dev.use_atomic) {
>- dev.req = drmModeAtomicAlloc();
>
> if (set_preferred || (count && plane_count)) {
> uint64_t cap = 0;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH libdrm v3 2/2] tests/modetest: Add support for writeback connector
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 2/2] tests/modetest: Add support for writeback connector Jessica Zhang
@ 2022-09-02 6:40 ` Dmitry Baryshkov
2022-09-30 21:44 ` Jessica Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2022-09-02 6:40 UTC (permalink / raw)
To: Jessica Zhang, dri-devel; +Cc: hoegsberg, Rohith Iyer, quic_abhinavk
On 2 September 2022 02:09:24 GMT+03:00, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>From: Rohith Iyer <quic_rohiiyer@quicinc.com>
>
>Add writeback support to modetest with the below options:
>
>- Passing in -c will now also show the writeback connector
>
>- Test a built-in mode on writeback connector
>
>- Test a custom mode from user input on writeback connector
> Usage: "./modetest -M msm -x <connector_id>:<mode_parameters>
> -a -P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24."
> Refer to --help for exact syntax
>
>- Dump the writeback output buffer to bitstream
> Usage: "./modetest -M msm -s <connector_id>:<widthxheight>
> -a -o <filepath>
> -P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24"
>
>This currently supports a singular writeback connector.
>
>Changes made in V2:
>- Added helper method that checks if user pipe has writeback connector
>- Added error message for dump flag if no writeback connector is found
>- Polls on the writeback fence fd until writeback is complete
>
>Changes made in V3:
>- Resolved compiler warnings
>- Defined ETIME to ETIMEDOUT in cases where ETIME is undefined
>
>Co-developed-by: Rohith Iyer <quic_rohiiyer@quicinc.com>
>Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>---
> tests/modetest/buffers.c | 19 ++++
> tests/modetest/buffers.h | 1 +
> tests/modetest/modetest.c | 183 ++++++++++++++++++++++++++++++++++----
> 3 files changed, 184 insertions(+), 19 deletions(-)
>
>diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
>index 8a8d9e01..279d7b28 100644
>--- a/tests/modetest/buffers.c
>+++ b/tests/modetest/buffers.c
>@@ -353,3 +353,22 @@ void bo_destroy(struct bo *bo)
>
> free(bo);
> }
>+
>+void bo_dump(struct bo *bo, const char *filename)
>+{
>+ FILE *fp;
>+
>+ if (!bo || !filename)
>+ return;
>+
>+ fp = fopen(filename, "wb");
>+ if (fp) {
>+ void *addr;
>+
>+ bo_map(bo, &addr);
>+ printf("Dumping buffer %p to file %s.\n", bo->ptr, filename);
>+ fwrite(bo->ptr, 1, bo->size, fp);
>+ bo_unmap(bo);
>+ fclose(fp);
Any chance of using libpng, libungif or at the very least implementing minimal ppm or bmp support here? Dumping to the image file would help a lot.
>+ }
>+}
>diff --git a/tests/modetest/buffers.h b/tests/modetest/buffers.h
>index 7f95396b..cbd54e9e 100644
>--- a/tests/modetest/buffers.h
>+++ b/tests/modetest/buffers.h
>@@ -36,5 +36,6 @@ struct bo *bo_create(int fd, unsigned int format,
> unsigned int handles[4], unsigned int pitches[4],
> unsigned int offsets[4], enum util_fill_pattern pattern);
> void bo_destroy(struct bo *bo);
>+void bo_dump(struct bo *bo, const char *filename);
>
> #endif
>diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
>index 2c31c4fc..8073d143 100644
>--- a/tests/modetest/modetest.c
>+++ b/tests/modetest/modetest.c
>@@ -68,8 +68,13 @@
> #include "buffers.h"
> #include "cursor.h"
>
>+#ifndef ETIME
>+#define ETIME ETIMEDOUT
>+#endif
ETIME is on its way to obsolescence. Please use ETIMEDOUT. See https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
>+
> static enum util_fill_pattern primary_fill = UTIL_PATTERN_SMPTE;
> static enum util_fill_pattern secondary_fill = UTIL_PATTERN_TILES;
>+static enum util_fill_pattern plain_fill = UTIL_PATTERN_PLAIN;
>
> struct crtc {
> drmModeCrtc *crtc;
>@@ -128,6 +133,7 @@ struct device {
>
> int use_atomic;
> drmModeAtomicReq *req;
>+ int32_t writeback_fence_fd;
> };
>
> static inline int64_t U642I64(uint64_t val)
>@@ -135,6 +141,11 @@ static inline int64_t U642I64(uint64_t val)
> return (int64_t)*((int64_t *)&val);
> }
>
>+static inline uint64_t to_user_pointer(const void *ptr)
>+{
>+ return (uintptr_t)ptr;
>+}
>+
> static float mode_vrefresh(drmModeModeInfo *mode)
> {
> return mode->clock * 1000.00
>@@ -811,6 +822,10 @@ struct pipe_arg {
> struct crtc *crtc;
> unsigned int fb_id[2], current_fb_id;
> struct timeval start;
>+ unsigned int out_fb_id;
>+ struct bo *out_bo;
>+ bool custom;
>+ bool dump;
>
> int swap_count;
> };
>@@ -917,27 +932,43 @@ static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe)
> return &dev->resources->crtcs[crtc_idx - 1];
> }
>
>+static int parse_mode_string(char *mode_string, drmModeModeInfo *user_mode)
>+{
>+ return sscanf(mode_string, "%u,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%u,%s",
>+ &user_mode->clock, &user_mode->hdisplay, &user_mode->hsync_start,
>+ &user_mode->hsync_end, &user_mode->htotal, &user_mode->hskew,
>+ &user_mode->vdisplay, &user_mode->vsync_start, &user_mode->vsync_end,
>+ &user_mode->vtotal, &user_mode->vscan, &user_mode->vrefresh,
>+ user_mode->name);
>+}
>+
> static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
I'd instead rename this to pipe_find_crtc_and_builtin_mode to limit the indent level to remain sane.
Then define new function supporting builtin and custom modes.
> {
> drmModeModeInfo *mode = NULL;
> int i;
>+ static drmModeModeInfo user_mode;
Minor nit. If this a global variable, defining it so is more obvious compared to the hidden static global.
>
>- pipe->mode = NULL;
>-
>- for (i = 0; i < (int)pipe->num_cons; i++) {
>- mode = connector_find_mode(dev, pipe->con_ids[i],
>- pipe->mode_str, pipe->vrefresh);
>- if (mode == NULL) {
>- if (pipe->vrefresh)
>- fprintf(stderr,
>- "failed to find mode "
>- "\"%s-%.2fHz\" for connector %s\n",
>- pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
>- else
>- fprintf(stderr,
>- "failed to find mode \"%s\" for connector %s\n",
>- pipe->mode_str, pipe->cons[i]);
>+ if (pipe->custom) {
>+ if (parse_mode_string(pipe->mode_str, &user_mode) == 13)
>+ mode = &user_mode;
>+ else
> return -EINVAL;
>+ } else {
>+ for (i = 0; i < (int)pipe->num_cons; i++) {
>+ mode = connector_find_mode(dev, pipe->con_ids[i],
>+ pipe->mode_str, pipe->vrefresh);
>+ if (mode == NULL) {
>+ if (pipe->vrefresh)
>+ fprintf(stderr,
>+ "failed to find mode "
>+ "\"%s-%.2fHz\" for connector %s\n",
>+ pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
>+ else
>+ fprintf(stderr,
>+ "failed to find mode \"%s\" for connector %s\n",
>+ pipe->mode_str, pipe->cons[i]);
>+ return -EINVAL;
>+ }
> }
> }
>
>@@ -1441,6 +1472,24 @@ static int pipe_resolve_connectors(struct device *dev, struct pipe_arg *pipe)
> return 0;
> }
>
>+static bool pipe_has_writeback_connector(struct device *dev, struct pipe_arg *pipes,
>+ unsigned int count)
>+{
>+ drmModeConnector *connector;
>+ unsigned int i, j;
>+
>+ for (j = 0; j < count; j++) {
>+ struct pipe_arg *pipe = &pipes[j];
>+
>+ for (i = 0; i < pipe->num_cons; i++) {
>+ connector = get_connector_by_id(dev, pipe->con_ids[i]);
>+ if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>+ return true;
>+ }
>+ }
>+ return false;
>+}
>+
> static int pipe_attempt_connector(struct device *dev, drmModeConnector *con,
> struct pipe_arg *pipe)
> {
>@@ -1662,6 +1711,70 @@ static void set_mode(struct device *dev, struct pipe_arg *pipes, unsigned int co
> }
> }
>
>+static void writeback_config(struct device *dev, struct pipe_arg *pipes, unsigned int count)
>+{
>+ drmModeConnector *connector;
>+ unsigned int i, j;
>+
>+ for (j = 0; j < count; j++) {
>+ struct pipe_arg *pipe = &pipes[j];
>+
>+ for (i = 0; i < pipe->num_cons; i++) {
>+ connector = get_connector_by_id(dev, pipe->con_ids[i]);
>+ if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) {
>+ bo_fb_create(dev->fd, pipes[j].fourcc, pipe->mode->hdisplay,
>+ pipe->mode->vdisplay, plain_fill, &pipe->out_bo,
>+ &pipe->out_fb_id);
>+ add_property(dev, pipe->con_ids[i], "WRITEBACK_FB_ID",
>+ pipe->out_fb_id);
>+ add_property(dev, pipe->con_ids[i], "WRITEBACK_OUT_FENCE_PTR",
>+ to_user_pointer(&dev->writeback_fence_fd));
>+ }
>+ }
>+ }
>+}
>+
>+static int poll_writeback_fence(int fd, int timeout)
>+{
>+ struct pollfd fds = { fd, POLLIN };
>+ int ret;
>+
>+ do {
>+ ret = poll(&fds, 1, timeout);
>+ if (ret > 0) {
>+ if (fds.revents & (POLLERR | POLLNVAL))
>+ return -EINVAL;
>+
>+ return 0;
>+ } else if (ret == 0) {
>+ return -ETIME;
>+ } else {
>+ ret = -errno;
>+ if (ret == -EINTR || ret == -EAGAIN)
>+ continue;
>+ return ret;
>+ }
>+ } while (1);
>+
>+}
>+
>+static void dump_output_fb(struct device *dev, struct pipe_arg *pipes, char *dump_path,
>+ unsigned int count)
>+{
>+ drmModeConnector *connector;
>+ unsigned int i, j;
>+
>+ for (j = 0; j < count; j++) {
>+ struct pipe_arg *pipe = &pipes[j];
>+
>+ for (i = 0; i < pipe->num_cons; i++) {
>+ connector = get_connector_by_id(dev, pipe->con_ids[i]);
>+ if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>+ bo_dump(pipe->out_bo, dump_path);
>+ }
>+ }
>+}
>+
> static void atomic_clear_mode(struct device *dev, struct pipe_arg *pipes, unsigned int count)
> {
> unsigned int i;
>@@ -1990,7 +2103,7 @@ static void parse_fill_patterns(char *arg)
>
> static void usage(char *name)
> {
>- fprintf(stderr, "usage: %s [-acDdefMPpsCvrw]\n", name);
>+ fprintf(stderr, "usage: %s [-acDdefMPpsCvrwx]\n", name);
>
> fprintf(stderr, "\n Query options:\n\n");
> fprintf(stderr, "\t-c\tlist connectors\n");
>@@ -2000,7 +2113,12 @@ static void usage(char *name)
>
> fprintf(stderr, "\n Test options:\n\n");
> fprintf(stderr, "\t-P <plane_id>@<crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
>- fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:[#<mode index>]<mode>[-<vrefresh>][@<format>]\tset a mode\n");
>+ fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:[#<mode index>]");
>+ fprintf(stderr, "<mode>[-<vrefresh>][@<format>]\tset a built-in mode\n");
>+ fprintf(stderr, "\t-x <connector_id>[@<crtc_id>]:<clock,hdisplay,hsyncstart,hsyncend,");
>+ fprintf(stderr, "htotal,hskew,vdisplay,vsyncstart,vsyncend,vtotal,vscan,");
Does this work for non-WB connectors?
>+ fprintf(stderr, "vrefresh,name>\tset a custom mode\n");
>+ fprintf(stderr, "\t-o <desired file path> \t Dump writeback output buffer to file\n");
> fprintf(stderr, "\t-C\ttest hw cursor\n");
> fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
> fprintf(stderr, "\t-r\tset the preferred mode for all connectors\n");
>@@ -2017,7 +2135,7 @@ static void usage(char *name)
> exit(0);
> }
>
>-static char optstr[] = "acdD:efF:M:P:ps:Cvrw:";
>+static const char optstr[] = "acdD:efF:M:o:P:ps:Cvrw:x:";
>
> int main(int argc, char **argv)
> {
>@@ -2040,6 +2158,7 @@ int main(int argc, char **argv)
> struct property_arg *prop_args = NULL;
> unsigned int args = 0;
> int ret;
>+ char *dump_path;
>
> memset(&dev, 0, sizeof dev);
>
>@@ -2078,6 +2197,10 @@ int main(int argc, char **argv)
> /* Preserve the default behaviour of dumping all information. */
> args--;
> break;
>+ case 'o':
>+ pipe_args->dump = true;
>+ dump_path = optarg;
>+ break;
> case 'P':
> plane_args = realloc(plane_args,
> (plane_count + 1) * sizeof *plane_args);
>@@ -2096,6 +2219,7 @@ int main(int argc, char **argv)
> crtcs = 1;
> planes = 1;
> break;
>+ case 'x':
> case 's':
> pipe_args = realloc(pipe_args,
> (count + 1) * sizeof *pipe_args);
>@@ -2107,7 +2231,7 @@ int main(int argc, char **argv)
>
> if (parse_connector(&pipe_args[count], optarg) < 0)
> usage(argv[0]);
>-
>+ pipe_args->custom = (c == 'x');
> count++;
> break;
> case 'C':
>@@ -2163,6 +2287,7 @@ int main(int argc, char **argv)
>
> if (use_atomic) {
> ret = drmSetClientCap(dev.fd, DRM_CLIENT_CAP_ATOMIC, 1);
>+ drmSetClientCap(dev.fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
Only if dump was requested?
> if (ret) {
> fprintf(stderr, "no atomic modesetting support: %s\n", strerror(errno));
> drmClose(dev.fd);
>@@ -2206,6 +2331,12 @@ int main(int argc, char **argv)
> if (set_preferred || count)
> set_mode(&dev, pipe_args, count);
>
>+ if (pipe_has_writeback_connector(&dev, pipe_args, count))
>+ writeback_config(&dev, pipe_args, count);
>+ else if (pipe_args->dump)
>+ fprintf(stderr,
>+ "No writeback connector found - dump will be skipped.\n");
>+
> if (plane_count)
> atomic_set_planes(&dev, plane_args, plane_count, false);
>
>@@ -2215,6 +2346,20 @@ int main(int argc, char **argv)
> return 1;
> }
>
>+ /*
>+ * Since only writeback connectors have an output fb, this should only be
>+ * called for writeback.
>+ */
>+ if (pipe_has_writeback_connector(&dev, pipe_args, count)) {
>+ ret = poll_writeback_fence(dev.writeback_fence_fd, 1000);
>+ if (ret)
>+ fprintf(stderr,
>+ "Poll for writeback error: %d. Skipping Dump.\n",
>+ ret);
>+ else if (pipe_args->dump)
>+ dump_output_fb(&dev, pipe_args, dump_path, count);
>+ }
>+
> if (test_vsync)
> atomic_test_page_flip(&dev, pipe_args, plane_args, plane_count);
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH libdrm v3 2/2] tests/modetest: Add support for writeback connector
2022-09-02 6:40 ` Dmitry Baryshkov
@ 2022-09-30 21:44 ` Jessica Zhang
0 siblings, 0 replies; 6+ messages in thread
From: Jessica Zhang @ 2022-09-30 21:44 UTC (permalink / raw)
To: Dmitry Baryshkov; +Cc: Rohith Iyer, quic_abhinavk, dri-devel, hoegsberg
Hi Dmitry,
On 9/1/2022 11:40 PM, Dmitry Baryshkov wrote:
>
>
> On 2 September 2022 02:09:24 GMT+03:00, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>> From: Rohith Iyer <quic_rohiiyer@quicinc.com>
>>
>> Add writeback support to modetest with the below options:
>>
>> - Passing in -c will now also show the writeback connector
>>
>> - Test a built-in mode on writeback connector
>>
>> - Test a custom mode from user input on writeback connector
>> Usage: "./modetest -M msm -x <connector_id>:<mode_parameters>
>> -a -P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24."
>> Refer to --help for exact syntax
>>
>> - Dump the writeback output buffer to bitstream
>> Usage: "./modetest -M msm -s <connector_id>:<widthxheight>
>> -a -o <filepath>
>> -P <plane_id>@<crtc_id>:<widthxheight>+0+0@RG24"
>>
>> This currently supports a singular writeback connector.
>>
>> Changes made in V2:
>> - Added helper method that checks if user pipe has writeback connector
>> - Added error message for dump flag if no writeback connector is found
>> - Polls on the writeback fence fd until writeback is complete
>>
>> Changes made in V3:
>> - Resolved compiler warnings
>> - Defined ETIME to ETIMEDOUT in cases where ETIME is undefined
>>
>> Co-developed-by: Rohith Iyer <quic_rohiiyer@quicinc.com>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>> tests/modetest/buffers.c | 19 ++++
>> tests/modetest/buffers.h | 1 +
>> tests/modetest/modetest.c | 183 ++++++++++++++++++++++++++++++++++----
>> 3 files changed, 184 insertions(+), 19 deletions(-)
>>
>> diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
>> index 8a8d9e01..279d7b28 100644
>> --- a/tests/modetest/buffers.c
>> +++ b/tests/modetest/buffers.c
>> @@ -353,3 +353,22 @@ void bo_destroy(struct bo *bo)
>>
>> free(bo);
>> }
>> +
>> +void bo_dump(struct bo *bo, const char *filename)
>> +{
>> + FILE *fp;
>> +
>> + if (!bo || !filename)
>> + return;
>> +
>> + fp = fopen(filename, "wb");
>> + if (fp) {
>> + void *addr;
>> +
>> + bo_map(bo, &addr);
>> + printf("Dumping buffer %p to file %s.\n", bo->ptr, filename);
>> + fwrite(bo->ptr, 1, bo->size, fp);
>> + bo_unmap(bo);
>> + fclose(fp);
>
> Any chance of using libpng, libungif or at the very least implementing minimal ppm or bmp support here? Dumping to the image file would help a lot.
I can try checking out libpng, but it will take a while to implement.
Would it be alright if we kept the raw dump for now then submit a
separate series for adding libpng support?
Since this series adds more than just the dump feature, I want to avoid
having this block the rest of the patch from being merged unless
absolutely necessary.
>
>> + }
>> +}
>> diff --git a/tests/modetest/buffers.h b/tests/modetest/buffers.h
>> index 7f95396b..cbd54e9e 100644
>> --- a/tests/modetest/buffers.h
>> +++ b/tests/modetest/buffers.h
>> @@ -36,5 +36,6 @@ struct bo *bo_create(int fd, unsigned int format,
>> unsigned int handles[4], unsigned int pitches[4],
>> unsigned int offsets[4], enum util_fill_pattern pattern);
>> void bo_destroy(struct bo *bo);
>> +void bo_dump(struct bo *bo, const char *filename);
>>
>> #endif
>> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
>> index 2c31c4fc..8073d143 100644
>> --- a/tests/modetest/modetest.c
>> +++ b/tests/modetest/modetest.c
>> @@ -68,8 +68,13 @@
>> #include "buffers.h"
>> #include "cursor.h"
>>
>> +#ifndef ETIME
>> +#define ETIME ETIMEDOUT
>> +#endif
>
> ETIME is on its way to obsolescence. Please use ETIMEDOUT. See https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
Ah, got it. Had originally used ETIME to match the IGT implementation
[1], but will use ETIMEDOUT instead.
>
>> +
>> static enum util_fill_pattern primary_fill = UTIL_PATTERN_SMPTE;
>> static enum util_fill_pattern secondary_fill = UTIL_PATTERN_TILES;
>> +static enum util_fill_pattern plain_fill = UTIL_PATTERN_PLAIN;
>>
>> struct crtc {
>> drmModeCrtc *crtc;
>> @@ -128,6 +133,7 @@ struct device {
>>
>> int use_atomic;
>> drmModeAtomicReq *req;
>> + int32_t writeback_fence_fd;
>> };
>>
>> static inline int64_t U642I64(uint64_t val)
>> @@ -135,6 +141,11 @@ static inline int64_t U642I64(uint64_t val)
>> return (int64_t)*((int64_t *)&val);
>> }
>>
>> +static inline uint64_t to_user_pointer(const void *ptr)
>> +{
>> + return (uintptr_t)ptr;
>> +}
>> +
>> static float mode_vrefresh(drmModeModeInfo *mode)
>> {
>> return mode->clock * 1000.00
>> @@ -811,6 +822,10 @@ struct pipe_arg {
>> struct crtc *crtc;
>> unsigned int fb_id[2], current_fb_id;
>> struct timeval start;
>> + unsigned int out_fb_id;
>> + struct bo *out_bo;
>> + bool custom;
>> + bool dump;
>>
>> int swap_count;
>> };
>> @@ -917,27 +932,43 @@ static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe)
>> return &dev->resources->crtcs[crtc_idx - 1];
>> }
>>
>> +static int parse_mode_string(char *mode_string, drmModeModeInfo *user_mode)
>> +{
>> + return sscanf(mode_string, "%u,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%u,%s",
>> + &user_mode->clock, &user_mode->hdisplay, &user_mode->hsync_start,
>> + &user_mode->hsync_end, &user_mode->htotal, &user_mode->hskew,
>> + &user_mode->vdisplay, &user_mode->vsync_start, &user_mode->vsync_end,
>> + &user_mode->vtotal, &user_mode->vscan, &user_mode->vrefresh,
>> + user_mode->name);
>> +}
>> +
>> static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
>
> I'd instead rename this to pipe_find_crtc_and_builtin_mode to limit the indent level to remain sane.
>
> Then define new function supporting builtin and custom modes.
Sounds good.
>
>> {
>> drmModeModeInfo *mode = NULL;
>> int i;
>> + static drmModeModeInfo user_mode;
>
> Minor nit. If this a global variable, defining it so is more obvious compared to the hidden static global.
Noted -- will move to top outside of method.
>
>>
>> - pipe->mode = NULL;
>> -
>> - for (i = 0; i < (int)pipe->num_cons; i++) {
>> - mode = connector_find_mode(dev, pipe->con_ids[i],
>> - pipe->mode_str, pipe->vrefresh);
>> - if (mode == NULL) {
>> - if (pipe->vrefresh)
>> - fprintf(stderr,
>> - "failed to find mode "
>> - "\"%s-%.2fHz\" for connector %s\n",
>> - pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
>> - else
>> - fprintf(stderr,
>> - "failed to find mode \"%s\" for connector %s\n",
>> - pipe->mode_str, pipe->cons[i]);
>> + if (pipe->custom) {
>> + if (parse_mode_string(pipe->mode_str, &user_mode) == 13)
>> + mode = &user_mode;
>> + else
>> return -EINVAL;
>> + } else {
>> + for (i = 0; i < (int)pipe->num_cons; i++) {
>> + mode = connector_find_mode(dev, pipe->con_ids[i],
>> + pipe->mode_str, pipe->vrefresh);
>> + if (mode == NULL) {
>> + if (pipe->vrefresh)
>> + fprintf(stderr,
>> + "failed to find mode "
>> + "\"%s-%.2fHz\" for connector %s\n",
>> + pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
>> + else
>> + fprintf(stderr,
>> + "failed to find mode \"%s\" for connector %s\n",
>> + pipe->mode_str, pipe->cons[i]);
>> + return -EINVAL;
>> + }
>> }
>> }
>>
>> @@ -1441,6 +1472,24 @@ static int pipe_resolve_connectors(struct device *dev, struct pipe_arg *pipe)
>> return 0;
>> }
>>
>> +static bool pipe_has_writeback_connector(struct device *dev, struct pipe_arg *pipes,
>> + unsigned int count)
>> +{
>> + drmModeConnector *connector;
>> + unsigned int i, j;
>> +
>> + for (j = 0; j < count; j++) {
>> + struct pipe_arg *pipe = &pipes[j];
>> +
>> + for (i = 0; i < pipe->num_cons; i++) {
>> + connector = get_connector_by_id(dev, pipe->con_ids[i]);
>> + if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>> + return true;
>> + }
>> + }
>> + return false;
>> +}
>> +
>> static int pipe_attempt_connector(struct device *dev, drmModeConnector *con,
>> struct pipe_arg *pipe)
>> {
>> @@ -1662,6 +1711,70 @@ static void set_mode(struct device *dev, struct pipe_arg *pipes, unsigned int co
>> }
>> }
>>
>> +static void writeback_config(struct device *dev, struct pipe_arg *pipes, unsigned int count)
>> +{
>> + drmModeConnector *connector;
>> + unsigned int i, j;
>> +
>> + for (j = 0; j < count; j++) {
>> + struct pipe_arg *pipe = &pipes[j];
>> +
>> + for (i = 0; i < pipe->num_cons; i++) {
>> + connector = get_connector_by_id(dev, pipe->con_ids[i]);
>> + if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) {
>> + bo_fb_create(dev->fd, pipes[j].fourcc, pipe->mode->hdisplay,
>> + pipe->mode->vdisplay, plain_fill, &pipe->out_bo,
>> + &pipe->out_fb_id);
>> + add_property(dev, pipe->con_ids[i], "WRITEBACK_FB_ID",
>> + pipe->out_fb_id);
>> + add_property(dev, pipe->con_ids[i], "WRITEBACK_OUT_FENCE_PTR",
>> + to_user_pointer(&dev->writeback_fence_fd));
>> + }
>> + }
>> + }
>> +}
>> +
>> +static int poll_writeback_fence(int fd, int timeout)
>> +{
>> + struct pollfd fds = { fd, POLLIN };
>> + int ret;
>> +
>> + do {
>> + ret = poll(&fds, 1, timeout);
>> + if (ret > 0) {
>> + if (fds.revents & (POLLERR | POLLNVAL))
>> + return -EINVAL;
>> +
>> + return 0;
>> + } else if (ret == 0) {
>> + return -ETIME;
>> + } else {
>> + ret = -errno;
>> + if (ret == -EINTR || ret == -EAGAIN)
>> + continue;
>> + return ret;
>> + }
>> + } while (1);
>> +
>> +}
>> +
>> +static void dump_output_fb(struct device *dev, struct pipe_arg *pipes, char *dump_path,
>> + unsigned int count)
>> +{
>> + drmModeConnector *connector;
>> + unsigned int i, j;
>> +
>> + for (j = 0; j < count; j++) {
>> + struct pipe_arg *pipe = &pipes[j];
>> +
>> + for (i = 0; i < pipe->num_cons; i++) {
>> + connector = get_connector_by_id(dev, pipe->con_ids[i]);
>> + if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>> + bo_dump(pipe->out_bo, dump_path);
>> + }
>> + }
>> +}
>> +
>> static void atomic_clear_mode(struct device *dev, struct pipe_arg *pipes, unsigned int count)
>> {
>> unsigned int i;
>> @@ -1990,7 +2103,7 @@ static void parse_fill_patterns(char *arg)
>>
>> static void usage(char *name)
>> {
>> - fprintf(stderr, "usage: %s [-acDdefMPpsCvrw]\n", name);
>> + fprintf(stderr, "usage: %s [-acDdefMPpsCvrwx]\n", name);
>>
>> fprintf(stderr, "\n Query options:\n\n");
>> fprintf(stderr, "\t-c\tlist connectors\n");
>> @@ -2000,7 +2113,12 @@ static void usage(char *name)
>>
>> fprintf(stderr, "\n Test options:\n\n");
>> fprintf(stderr, "\t-P <plane_id>@<crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
>> - fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:[#<mode index>]<mode>[-<vrefresh>][@<format>]\tset a mode\n");
>> + fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:[#<mode index>]");
>> + fprintf(stderr, "<mode>[-<vrefresh>][@<format>]\tset a built-in mode\n");
>> + fprintf(stderr, "\t-x <connector_id>[@<crtc_id>]:<clock,hdisplay,hsyncstart,hsyncend,");
>> + fprintf(stderr, "htotal,hskew,vdisplay,vsyncstart,vsyncend,vtotal,vscan,");
>
> Does this work for non-WB connectors?
No, testing custom modes is a WB-only feature... will add checks for it
and mention it in the help text.
>
>> + fprintf(stderr, "vrefresh,name>\tset a custom mode\n");
>
>
>> + fprintf(stderr, "\t-o <desired file path> \t Dump writeback output buffer to file\n");
>> fprintf(stderr, "\t-C\ttest hw cursor\n");
>> fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
>> fprintf(stderr, "\t-r\tset the preferred mode for all connectors\n");
>> @@ -2017,7 +2135,7 @@ static void usage(char *name)
>> exit(0);
>> }
>>
>> -static char optstr[] = "acdD:efF:M:P:ps:Cvrw:";
>> +static const char optstr[] = "acdD:efF:M:o:P:ps:Cvrw:x:";
>>
>> int main(int argc, char **argv)
>> {
>> @@ -2040,6 +2158,7 @@ int main(int argc, char **argv)
>> struct property_arg *prop_args = NULL;
>> unsigned int args = 0;
>> int ret;
>> + char *dump_path;
>>
>> memset(&dev, 0, sizeof dev);
>>
>> @@ -2078,6 +2197,10 @@ int main(int argc, char **argv)
>> /* Preserve the default behaviour of dumping all information. */
>> args--;
>> break;
>> + case 'o':
>> + pipe_args->dump = true;
>> + dump_path = optarg;
>> + break;
>> case 'P':
>> plane_args = realloc(plane_args,
>> (plane_count + 1) * sizeof *plane_args);
>> @@ -2096,6 +2219,7 @@ int main(int argc, char **argv)
>> crtcs = 1;
>> planes = 1;
>> break;
>> + case 'x':
>> case 's':
>> pipe_args = realloc(pipe_args,
>> (count + 1) * sizeof *pipe_args);
>> @@ -2107,7 +2231,7 @@ int main(int argc, char **argv)
>>
>> if (parse_connector(&pipe_args[count], optarg) < 0)
>> usage(argv[0]);
>> -
>> + pipe_args->custom = (c == 'x');
>> count++;
>> break;
>> case 'C':
>> @@ -2163,6 +2287,7 @@ int main(int argc, char **argv)
>>
>> if (use_atomic) {
>> ret = drmSetClientCap(dev.fd, DRM_CLIENT_CAP_ATOMIC, 1);
>> + drmSetClientCap(dev.fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
>
> Only if dump was requested?
Dumping the output is just an optional flag for testing the writeback
connector.
Thanks,
Jessica Zhang
>
>
>> if (ret) {
>> fprintf(stderr, "no atomic modesetting support: %s\n", strerror(errno));
>> drmClose(dev.fd);
>> @@ -2206,6 +2331,12 @@ int main(int argc, char **argv)
>> if (set_preferred || count)
>> set_mode(&dev, pipe_args, count);
>>
>> + if (pipe_has_writeback_connector(&dev, pipe_args, count))
>> + writeback_config(&dev, pipe_args, count);
>> + else if (pipe_args->dump)
>> + fprintf(stderr,
>> + "No writeback connector found - dump will be skipped.\n");
>> +
>> if (plane_count)
>> atomic_set_planes(&dev, plane_args, plane_count, false);
>>
>> @@ -2215,6 +2346,20 @@ int main(int argc, char **argv)
>> return 1;
>> }
>>
>> + /*
>> + * Since only writeback connectors have an output fb, this should only be
>> + * called for writeback.
>> + */
>> + if (pipe_has_writeback_connector(&dev, pipe_args, count)) {
>> + ret = poll_writeback_fence(dev.writeback_fence_fd, 1000);
>> + if (ret)
>> + fprintf(stderr,
>> + "Poll for writeback error: %d. Skipping Dump.\n",
>> + ret);
>> + else if (pipe_args->dump)
>> + dump_output_fb(&dev, pipe_args, dump_path, count);
>> + }
>> +
>> if (test_vsync)
>> atomic_test_page_flip(&dev, pipe_args, plane_args, plane_count);
>>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-30 21:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-01 23:09 [RESEND PATCH libdrm v3 0/2] Add Writeback Support for Modetest Jessica Zhang
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 1/2] tests/modetest: Allocate drmModeAtomicReq before setting properties Jessica Zhang
2022-09-02 6:10 ` Dmitry Baryshkov
2022-09-01 23:09 ` [RESEND PATCH libdrm v3 2/2] tests/modetest: Add support for writeback connector Jessica Zhang
2022-09-02 6:40 ` Dmitry Baryshkov
2022-09-30 21:44 ` Jessica Zhang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.