Hi Vincent,
Thank you for the patch.
On Tuesday 07 January 2014 09:55:40 Vincent ABRIOU wrote:
> When mode is selected we only give the name of the mode as parameter.
> But sometime, two different modes have the same name but not
> the same vrefresh frequency.
> This patch give the possibility to select a mode by its name
> and optionally by its refresh frequency.
>
> Signed-off-by: Vincent Abriou <vincent.abriou at st.com>
> ---
> tests/modetest/modetest.c | 38 ++++++++++++++++++++++++++++++--------
> 1 file changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 51c4e6d..259d580 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -693,6 +693,7 @@ struct pipe_arg {
> uint32_t crtc_id;
> char mode_str[64];
> char format_str[5];
> + unsigned int refresh_freq;
What about calling it vrefresh to match the drm_mode field name ?
> unsigned int fourcc;
> drmModeModeInfo *mode;
> struct crtc *crtc;
> @@ -714,7 +715,8 @@ struct plane_arg {
> };
>
> static drmModeModeInfo *
> -connector_find_mode(struct device *dev, uint32_t con_id, const char
> *mode_str)
> +connector_find_mode(struct device *dev, uint32_t con_id, const char
> *mode_str,
> + const unsigned int refresh_freq)
> {
> drmModeConnector *connector;
> drmModeModeInfo *mode;
> @@ -726,8 +728,16 @@ connector_find_mode(struct device *dev, uint32_t
> con_id, const char *mode_str)
>
> for (i = 0; i < connector->count_modes; i++) {
> mode = &connector->modes[i];
> - if (!strcmp(mode->name, mode_str))
> - return mode;
> + if (!strcmp(mode->name, mode_str)) {
> + if (refresh_freq == 0)
> + /* If the refresh frequency is not specified
> then return the
> + * first mode that match with the name */
> + return mode;
> + else if (mode->vrefresh == refresh_freq)
> + /* Else, return the mode that match the name
> and the
> specified
> + * refresh frequency */
> + return mode;
> + }
> }
>
> return NULL;
> @@ -789,7 +799,7 @@ static int pipe_find_crtc_and_mode(struct device *dev,
> struct pipe_arg *pipe)
>
> for (i = 0; i < (int)pipe->num_cons; i++) {
> mode = connector_find_mode(dev, pipe->con_ids[i],
> - pipe->mode_str);
> + pipe->mode_str, pipe->refresh_freq);
> if (mode == NULL) {
> fprintf(stderr,
> "failed to find mode \"%s\" for connector %u\n",
> @@ -1059,8 +1069,8 @@ static void set_mode(struct device *dev, struct
> pipe_arg *pipes, unsigned int co if (pipe->mode == NULL)
> continue;
>
> - printf("setting mode %s@%s on connectors ",
> - pipe->mode_str, pipe->format_str);
> + printf("setting mode %s-%dHz@%s on connectors ",
> + pipe->mode_str, pipe->mode->vrefresh, pipe->format_str);
> for (j = 0; j < pipe->num_cons; ++j)
> printf("%u, ", pipe->con_ids[j]);
> printf("crtc %d\n", pipe->crtc->crtc->crtc_id);
> @@ -1192,6 +1202,7 @@ static int parse_connector(struct pipe_arg *pipe,
> const char *arg) const char *p;
> char *endp;
>
> + pipe->refresh_freq = 0;
> pipe->crtc_id = (uint32_t)-1;
> strcpy(pipe->format_str, "XR24");
>
> @@ -1226,11 +1237,22 @@ static int parse_connector(struct pipe_arg *pipe,
> const char *arg)
>
> arg = endp + 1;
>
> - p = strchrnul(arg, '@');
> + /* Check if the refresh frequency is specified */
> + p = strchr(arg, '-');
> + if (p == NULL)
> + /* Else check if format is specified */
> + p = strchrnul(arg, '@');
Seems we need a strpbrknul :-)
Another option would be
/* Search for the vertical refresh frequency or the fourcc. */
p = strpbrk(arg, ":@");
if (p == NULL)
p = arg + strlen(arg);
I'm not sure which one would be more efficient.
> len = min(sizeof pipe->mode_str - 1, (unsigned int)(p - arg));
> strncpy(pipe->mode_str, arg, len);
> pipe->mode_str[len] = '\0';
>
> + if (*p == '-') {
> + pipe->refresh_freq = strtoul(p + 1, &endp, 10);
> + arg = endp;
> + /* Once done, check if format is specified */
> + p = strchrnul(arg, '@');
I think you can replace the strchrnul with a p = endp. There can't be anything
between the end of the refresh rate and the '@'.
> + }
> +
> if (*p == '@') {
> strncpy(pipe->format_str, p + 1, 4);
> pipe->format_str[4] = '\0';
> @@ -1323,7 +1345,7 @@ static void usage(char *name)
>
> fprintf(stderr, "\n Test options:\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>[@<format>]\tset a mode\n");
> + fprintf(stderr, "\t-s <connector_id>[,<connector_id>]
> [@<crtc_id>]:<mode>[-<refresh_freq>][@<format>]\tset a mode\n");
> fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
> fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
--
Regards,
Laurent Pinchart
