On 23 September 2015 at 03:45, Hyungwon Hwang <human.hwang@xxxxxxxxxxx> wrote:
> This patch adds support for atomic modeset. Using -a option, user can
> make modeset to use DRM_IOCTL_MODE_ATOMIC instead of legacy IOCTLs.
> Also, by using -w option, user can set the property as before.
>
> Signed-off-by: Hyungwon Hwang <human.hwang@xxxxxxxxxxx>
> ---
> tests/modetest/modetest.c | 273 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 265 insertions(+), 8 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 08ecf58..bc5a227 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -1477,25 +1477,32 @@ static int parse_property(struct property_arg *p, const char *arg)
>
> static void usage(char *name)
> {
> - fprintf(stderr, "usage: %s [-cDdefMPpsCvw]\n", name);
> + fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
> + fprintf(stderr, "\tA: supported in atomic modeset\n");
> + fprintf(stderr, "\tL: supported in legacy modeset\n");
>
> - fprintf(stderr, "\n Query options:\n\n");
> + fprintf(stderr, "\n Query options: [AL]\n\n");
> fprintf(stderr, "\t-c\tlist connectors\n");
> fprintf(stderr, "\t-e\tlist encoders\n");
> fprintf(stderr, "\t-f\tlist framebuffers\n");
> fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
>
> - fprintf(stderr, "\n Test options:\n\n");
> + fprintf(stderr, "\n Common Test options: [AL]\n\n");
> + fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
> +
> + fprintf(stderr, "\n Atomic Test options: [A]\n\n");
> + fprintf(stderr, "\t-a\tuse atomic modeset\n");
> +
> + fprintf(stderr, "\n Legacy test options: [L]\n\n");
> fprintf(stderr, "\t-P <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
> fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:<mode>[-<vrefresh>][@<format>]\tset a mode\n");
> fprintf(stderr, "\t-C\ttest hw cursor\n");
> fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
> - fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
>
> fprintf(stderr, "\n Generic options:\n\n");
> - fprintf(stderr, "\t-d\tdrop master after mode set\n");
> - fprintf(stderr, "\t-M module\tuse the given driver\n");
> - fprintf(stderr, "\t-D device\tuse the given device\n");
> + fprintf(stderr, "\t-d\tdrop master after mode set [L]\n");
> + fprintf(stderr, "\t-M module\tuse the given driver [AL]\n");
> + fprintf(stderr, "\t-D device\tuse the given device [AL]\n");
>
> fprintf(stderr, "\n\tDefault is to dump all info.\n");
> exit(0);
> @@ -1554,7 +1561,224 @@ static int pipe_resolve_connectors(struct device *dev, struct pipe_arg *pipe)
> return 0;
> }
>
> -static char optstr[] = "cdD:efM:P:ps:Cvw:";
> +static bool is_obj_id_in_prop_args(struct property_arg *prop_args,
const struct
> + unsigned int prop_count, uint32_t obj_id)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < prop_count; i++)
> + if (obj_id == prop_args[i].obj_id)
> + return true;
> +
> + return false;
> +}
> +
> +static int get_value_in_prop_args(struct property_arg *prop_args,
Ditto
> + unsigned int prop_count, uint32_t obj_id,
> + const char *name, uint64_t *out)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < prop_count; i++) {
> + if (prop_args[i].obj_id == obj_id &&
> + !strcmp(prop_args[i].name, name)) {
> + *out = prop_args[i].value;
> + return 0;
> + }
> + }
> +
> + return -1;
> +}
> +
> +static uint32_t get_plane_prop_id(struct resources *res, uint32_t obj_id,
Ditto
> + const char *name)
> +{
> + drmModePropertyRes *props_info;
> + struct plane *plane;
> + unsigned int i, j;
> +
> + for (i = 0; i < res->plane_res->count_planes; i++) {
> + plane = &res->planes[i];
> + if (plane->plane->plane_id != obj_id)
> + continue;
> +
> + for (j = 0; j < plane->props->count_props; j++) {
> + props_info = plane->props_info[j];
> + if (!strcmp(props_info->name, name))
> + return props_info->prop_id;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int allocate_fb(int fd, drmModeAtomicReqPtr req, struct resources *res,
req and res are unused.
> + uint32_t width, uint32_t height, uint32_t pixel_format,
> + int pattern, struct bo **bo, uint32_t *fb_id)
> +{
> + uint32_t handles[4] = {0}, pitches[4] = {0}, offsets[4] = {0};
> + int ret;
> +
> + *bo = bo_create(fd, pixel_format, width, height,
> + handles, pitches, offsets, pattern);
> + if (*bo == NULL) {
> + fprintf(stderr, "failed to create bo (%ux%u): %s\n",
> + width, height, strerror(errno));
> + return -1;
> + }
> +
> + ret = drmModeAddFB2(fd, width, height, pixel_format,
> + handles, pitches, offsets, fb_id, 0);
> + if (ret) {
> + fprintf(stderr, "failed to add fb (%ux%u): %s\n",
> + width, height, strerror(errno));
> + bo_destroy(*bo);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int allocate_fbs(struct device *dev, drmModeAtomicReqPtr req,
> + struct resources *res, struct property_arg *prop_args,
Constify res and prop_args ?
> + unsigned int prop_count, struct bo **bo, uint32_t *fb_id)
> +{
> + uint32_t plane_id, fb_obj_id, pixel_format;
> + uint64_t width, height;
> + int ret = 0, i;
> +
> + for (i = 0; i < (int)res->plane_res->count_planes; i++) {
Define i as unsigned, drop the cast and ...
> + plane_id = res->planes[i].plane->plane_id;
> + if (!is_obj_id_in_prop_args(prop_args, prop_count, plane_id))
> + continue;
> +
> + fb_obj_id = get_plane_prop_id(res, plane_id, "FB_ID");
> + if (!fb_obj_id) {
> + fprintf(stderr, "plane(%u) does not exist\n", plane_id);
> + goto err;
> + }
> +
> + ret = get_value_in_prop_args(prop_args, prop_count, plane_id,
> + "SRC_W", &width);
> + if (ret < 0) {
> + fprintf(stderr, "SRC_W for plane (%u) must be set\n", plane_id);
> + goto err;
> + }
> +
> + ret = get_value_in_prop_args(prop_args, prop_count, plane_id,
> + "SRC_H", &height);
> + if (ret < 0) {
> + fprintf(stderr, "SRC_H for plane (%u) must be set\n", plane_id);
> + goto err;
> + }
> +
> + pixel_format = DRM_FORMAT_XRGB8888;
> +
> + ret = allocate_fb(dev->fd, req, res, width, height, pixel_format,
> + PATTERN_SMPTE, &bo[i], &fb_id[i]);
> + if (ret < 0)
> + goto err;
> +
> + ret = drmModeAtomicAddProperty(req, plane_id, fb_obj_id, fb_id[i]);
> + if (ret < 0) {
> + fprintf(stderr, "failed to add atomic property for atomic modeset\n");
> + goto err;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + while (i > -1) {
... then use
while (i--) {
> + if (!fb_id[i])
> + continue;
> +
> + drmModeRmFB(dev->fd, fb_id[i]);
> + bo_destroy(bo[i]);
> + i--;
and drop this line.
> + }
> +
> + return ret;
> +}
> +
> +static void deallocate_fbs(int fd, int num_planes, uint32_t *fb_id, struct bo **bo)
> +{
> + int i;
> +
Make both i and num_planes unsigned ?
> + for (i = 0; i < num_planes; i++) {
> + if (!fb_id[i])
> + continue;
> +
> + drmModeRmFB(fd, fb_id[i]);
> + bo_destroy(bo[i]);
Shove the contents of the loop into deallocate_fb() to complement
allocate_fb() and use it here + allocate_fbs() error path ?
> + }
> +}
> +
> +static int atomic_modeset(struct device *dev, drmModeAtomicReqPtr req,
> + struct property_arg *prop_args, unsigned int prop_count)
> +{
> + unsigned int num_planes;
> + const char *obj_type = NULL;
You don't need this.
> + uint32_t flags = 0;
And you can use 0 directly where needed.
> + uint32_t *fb_id;
> + struct bo **bo;
> + int ret, i;
> +
> + for (i = 0; (unsigned int)i < prop_count; ++i) {
Why is i signed int ?
> + ret = get_prop_info(dev->resources, &prop_args[i], obj_type);
> + if (ret < 0)
> + return ret;
> +
> + ret = drmModeAtomicAddProperty(req, prop_args[i].obj_id,
> + prop_args[i].prop_id, prop_args[i].value);
> + if (ret < 0) {
Perhaps we should unwind all the state that we've accumulated (same
goes for the remaining error paths in this func), although I might be
pushing my luck with your patience.
> + fprintf(stderr, "failed to add property for atomic modeset (%d)\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + num_planes = dev->resources->plane_res->count_planes;
> +
> + bo = calloc(num_planes, sizeof(*bo));
> + if (!bo) {
> + fprintf(stderr, "failed to allocate memory fo bo array\n");
> + return -1;
> + }
> +
> + fb_id = calloc(num_planes, sizeof(*fb_id));
> + if (!fb_id) {
> + fprintf(stderr, "failed to allocate memory fo fb_id array\n");
> + free(bo);
> + return -1;
> + }
> +
> + ret = allocate_fbs(dev, req, dev->resources, prop_args, prop_count, bo, fb_id);
> + if (ret < 0) {
> + free(fb_id);
> + free(bo);
> + return ret;
> + }
> +
> + ret = drmModeAtomicCommit(dev->fd, req, flags, NULL);
> + if (ret < 0) {
> + fprintf(stderr, "failed to commit for atomic modeset\n");
Use labels rather than the ever increasing teardown ?
> + deallocate_fbs(dev->fd, num_planes, fb_id, bo);
> + free(fb_id);
> + free(bo);
> + return ret;
> + }
> +
> + getchar();
> +
> + deallocate_fbs(dev->fd, num_planes, fb_id, bo);
> + free(fb_id);
> + free(bo);
> +
> + return 0;
> +}
> +
> +static char optstr[] = "acdD:efM:P:ps:Cvw:";
>
> int main(int argc, char **argv)
> {
> @@ -1574,6 +1798,8 @@ int main(int argc, char **argv)
> struct pipe_arg *pipe_args = NULL;
> struct plane_arg *plane_args = NULL;
> struct property_arg *prop_args = NULL;
> + drmModeAtomicReqPtr req = NULL;
Initialisation isn't needed.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
