All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH libdrm] modetest: Use floating vrefresh while dumping mode
@ 2019-11-26 15:03 Devarsh Thakkar
  2019-11-26 15:15 ` Ville Syrjälä
  0 siblings, 1 reply; 4+ messages in thread
From: Devarsh Thakkar @ 2019-11-26 15:03 UTC (permalink / raw)
  To: dri-devel, ville.syrjala
  Cc: rsk, vcu-team, dshah, devarsh.thakkar, varunkum, satishna, rvisaval

Add inline function to derive floating value of vertical
refresh rate from pixel clock, horizontal total size and
vertical total size.

Use this function to find suitable mode having vrefresh
value which is matching with user provided vrefresh value.

If user doesn't provide any vrefresh value in args then
update vertical refresh rate value in pipe args using this
function so that it will be used for printing floating vrefresh
while dumping mode.

This will give more accurate picture to user for available modes
differentiated by floating vertical refresh rate and help user
select more appropriate mode using suitable refresh rate value.

Signed-off-by: Devarsh Thakkar <devarsh.thakkar@xilinx.com>
---
 tests/modetest/modetest.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index b4edfcb..80cf076 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -133,6 +133,12 @@ static inline int64_t U642I64(uint64_t val)
 	return (int64_t)*((int64_t *)&val);
 }
 
+static inline float get_floating_vrefresh(drmModeModeInfo *mode)
+{
+	return  mode->clock * 1000.00
+			/ (mode->htotal * mode->vtotal);
+}
+
 #define bit_name_fn(res)					\
 const char * res##_str(int type) {				\
 	unsigned int i;						\
@@ -210,9 +216,9 @@ static void dump_encoders(struct device *dev)
 
 static void dump_mode(drmModeModeInfo *mode)
 {
-	printf("  %s %d %d %d %d %d %d %d %d %d %d",
+	printf("  %s %.2f %d %d %d %d %d %d %d %d %d",
 	       mode->name,
-	       mode->vrefresh,
+	       get_floating_vrefresh(mode),
 	       mode->hdisplay,
 	       mode->hsync_start,
 	       mode->hsync_end,
@@ -823,12 +829,11 @@ struct plane_arg {
 
 static drmModeModeInfo *
 connector_find_mode(struct device *dev, uint32_t con_id, const char *mode_str,
-	const float vrefresh)
+	float *vrefresh)
 {
 	drmModeConnector *connector;
 	drmModeModeInfo *mode;
 	int i;
-	float mode_vrefresh;
 
 	connector = get_connector_by_id(dev, con_id);
 	if (!connector || !connector->count_modes)
@@ -837,16 +842,19 @@ 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)) {
-			/* If the vertical refresh frequency is not specified then return the
-			 * first mode that match with the name. Else, return the mode that match
-			 * the name and the specified vertical refresh frequency.
+			/* If the vertical refresh frequency is not specified
+			 * then return the first mode that match with the name
+			 * and update corresponding vrefresh in pipe_arg.
+			 * Else, return the mode that match the name and
+			 * the specified vertical refresh frequency.
 			 */
-			mode_vrefresh = mode->clock * 1000.00
-					/ (mode->htotal * mode->vtotal);
-			if (vrefresh == 0)
+			if (*vrefresh == 0) {
+				*vrefresh = get_floating_vrefresh(mode);
 				return mode;
-			else if (fabs(mode_vrefresh - vrefresh) < 0.005)
+			} else if (fabs(get_floating_vrefresh(mode)
+				   - *vrefresh) < 0.005) {
 				return mode;
+			}
 		}
 	}
 
@@ -909,11 +917,11 @@ 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->vrefresh);
+					   pipe->mode_str, &pipe->vrefresh);
 		if (mode == NULL) {
 			fprintf(stderr,
-				"failed to find mode \"%s\" for connector %s\n",
-				pipe->mode_str, pipe->cons[i]);
+			"failed to find mode \"%s-%.2fHz\" for connector %s\n",
+			 pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
 			return -EINVAL;
 		}
 	}
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH libdrm] modetest: Use floating vrefresh while dumping mode
  2019-11-26 15:03 [PATCH libdrm] modetest: Use floating vrefresh while dumping mode Devarsh Thakkar
@ 2019-11-26 15:15 ` Ville Syrjälä
  2019-11-27  7:28   ` Devarsh Thakkar
  0 siblings, 1 reply; 4+ messages in thread
From: Ville Syrjälä @ 2019-11-26 15:15 UTC (permalink / raw)
  To: Devarsh Thakkar
  Cc: rsk, vcu-team, dshah, dri-devel, varunkum, satishna, rvisaval

On Tue, Nov 26, 2019 at 07:03:58AM -0800, Devarsh Thakkar wrote:
> Add inline function to derive floating value of vertical
> refresh rate from pixel clock, horizontal total size and
> vertical total size.
> 
> Use this function to find suitable mode having vrefresh
> value which is matching with user provided vrefresh value.
> 
> If user doesn't provide any vrefresh value in args then
> update vertical refresh rate value in pipe args using this
> function so that it will be used for printing floating vrefresh
> while dumping mode.
> 
> This will give more accurate picture to user for available modes
> differentiated by floating vertical refresh rate and help user
> select more appropriate mode using suitable refresh rate value.
> 
> Signed-off-by: Devarsh Thakkar <devarsh.thakkar@xilinx.com>
> ---
>  tests/modetest/modetest.c | 36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index b4edfcb..80cf076 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -133,6 +133,12 @@ static inline int64_t U642I64(uint64_t val)
>  	return (int64_t)*((int64_t *)&val);
>  }
>  
> +static inline float get_floating_vrefresh(drmModeModeInfo *mode)

Drop the 'inline'. I'd call it just mode_vrefresh() or somehting like
that.

> +{
> +	return  mode->clock * 1000.00
> +			/ (mode->htotal * mode->vtotal);
> +}
> +
>  #define bit_name_fn(res)					\
>  const char * res##_str(int type) {				\
>  	unsigned int i;						\
> @@ -210,9 +216,9 @@ static void dump_encoders(struct device *dev)
>  
>  static void dump_mode(drmModeModeInfo *mode)
>  {
> -	printf("  %s %d %d %d %d %d %d %d %d %d %d",
> +	printf("  %s %.2f %d %d %d %d %d %d %d %d %d",
>  	       mode->name,
> -	       mode->vrefresh,
> +	       get_floating_vrefresh(mode),
>  	       mode->hdisplay,
>  	       mode->hsync_start,
>  	       mode->hsync_end,
> @@ -823,12 +829,11 @@ struct plane_arg {
>  
>  static drmModeModeInfo *
>  connector_find_mode(struct device *dev, uint32_t con_id, const char *mode_str,
> -	const float vrefresh)
> +	float *vrefresh)
>  {
>  	drmModeConnector *connector;
>  	drmModeModeInfo *mode;
>  	int i;
> -	float mode_vrefresh;
>  
>  	connector = get_connector_by_id(dev, con_id);
>  	if (!connector || !connector->count_modes)
> @@ -837,16 +842,19 @@ 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)) {
> -			/* If the vertical refresh frequency is not specified then return the
> -			 * first mode that match with the name. Else, return the mode that match
> -			 * the name and the specified vertical refresh frequency.
> +			/* If the vertical refresh frequency is not specified
> +			 * then return the first mode that match with the name
> +			 * and update corresponding vrefresh in pipe_arg.
> +			 * Else, return the mode that match the name and
> +			 * the specified vertical refresh frequency.
>  			 */
> -			mode_vrefresh = mode->clock * 1000.00
> -					/ (mode->htotal * mode->vtotal);
> -			if (vrefresh == 0)
> +			if (*vrefresh == 0) {
> +				*vrefresh = get_floating_vrefresh(mode);
>  				return mode;
> -			else if (fabs(mode_vrefresh - vrefresh) < 0.005)
> +			} else if (fabs(get_floating_vrefresh(mode)
> +				   - *vrefresh) < 0.005) {
>  				return mode;
> +			}
>  		}
>  	}
>  
> @@ -909,11 +917,11 @@ 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->vrefresh);
> +					   pipe->mode_str, &pipe->vrefresh);
>  		if (mode == NULL) {
>  			fprintf(stderr,
> -				"failed to find mode \"%s\" for connector %s\n",
> -				pipe->mode_str, pipe->cons[i]);
> +			"failed to find mode \"%s-%.2fHz\" for connector %s\n",
> +			 pipe->mode_str, pipe->vrefresh, pipe->cons[i]);

We didn't find any mode so having connector_find_mode() update
vrefresh for us is nonsense.

>  			return -EINVAL;
>  		}
>  	}
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* RE: [PATCH libdrm] modetest: Use floating vrefresh while dumping mode
  2019-11-26 15:15 ` Ville Syrjälä
@ 2019-11-27  7:28   ` Devarsh Thakkar
  2019-11-27 12:59     ` Ville Syrjälä
  0 siblings, 1 reply; 4+ messages in thread
From: Devarsh Thakkar @ 2019-11-27  7:28 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Ranganathan Sk, vcu-team, Dhaval Rajeshbhai Shah, dri-devel,
	Varunkumar Allagadapa, Satish Kumar Nagireddy, Rohit Visavalia

Thanks for the review Ville, please see inline:

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: 26 November 2019 07:15
> To: Devarsh Thakkar <DEVARSHT@xilinx.com>
> Cc: dri-devel@lists.freedesktop.org; Hyun Kwon <hyunk@xilinx.com>; vcu-
> team <vcu-team@xilinx.com>; Ranganathan Sk <rsk@xilinx.com>; Dhaval
> Rajeshbhai Shah <dshah@xilinx.com>; Satish Kumar Nagireddy
> <SATISHNA@xilinx.com>; Varunkumar Allagadapa <VARUNKUM@xilinx.com>;
> Rohit Visavalia <RVISAVAL@xilinx.com>
> Subject: Re: [PATCH libdrm] modetest: Use floating vrefresh while dumping
> mode
> 
> EXTERNAL EMAIL
> 
> On Tue, Nov 26, 2019 at 07:03:58AM -0800, Devarsh Thakkar wrote:
> > Add inline function to derive floating value of vertical refresh rate
> > from pixel clock, horizontal total size and vertical total size.
> >
> > Use this function to find suitable mode having vrefresh value which is
> > matching with user provided vrefresh value.
> >
> > If user doesn't provide any vrefresh value in args then update
> > vertical refresh rate value in pipe args using this function so that
> > it will be used for printing floating vrefresh while dumping mode.
> >
> > This will give more accurate picture to user for available modes
> > differentiated by floating vertical refresh rate and help user select
> > more appropriate mode using suitable refresh rate value.
> >
> > Signed-off-by: Devarsh Thakkar <devarsh.thakkar@xilinx.com>
> > ---
> >  tests/modetest/modetest.c | 36 ++++++++++++++++++++++--------------
> >  1 file changed, 22 insertions(+), 14 deletions(-)
> >
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index b4edfcb..80cf076 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -133,6 +133,12 @@ static inline int64_t U642I64(uint64_t val)
> >       return (int64_t)*((int64_t *)&val);  }
> >
> > +static inline float get_floating_vrefresh(drmModeModeInfo *mode)
> 
> Drop the 'inline'. I'd call it just mode_vrefresh() or somehting like that.
> 

Yes I do agree that most compilers will substitute function body if needed (and I do see that in generated assembly even without inline) but I still thought better to add inline keyword just to give the hint to compiler in case if it doesn't.
Any specific reason or issues if we use inline ?


> > +{
> > +     return  mode->clock * 1000.00
> > +                     / (mode->htotal * mode->vtotal); }
> > +
> >  #define bit_name_fn(res)                                     \
> >  const char * res##_str(int type) {                           \
> >       unsigned int i;                                         \
> > @@ -210,9 +216,9 @@ static void dump_encoders(struct device *dev)
> >
> >  static void dump_mode(drmModeModeInfo *mode)  {
> > -     printf("  %s %d %d %d %d %d %d %d %d %d %d",
> > +     printf("  %s %.2f %d %d %d %d %d %d %d %d %d",
> >              mode->name,
> > -            mode->vrefresh,
> > +            get_floating_vrefresh(mode),
> >              mode->hdisplay,
> >              mode->hsync_start,
> >              mode->hsync_end,
> > @@ -823,12 +829,11 @@ struct plane_arg {
> >
> >  static drmModeModeInfo *
> >  connector_find_mode(struct device *dev, uint32_t con_id, const char
> *mode_str,
> > -     const float vrefresh)
> > +     float *vrefresh)
> >  {
> >       drmModeConnector *connector;
> >       drmModeModeInfo *mode;
> >       int i;
> > -     float mode_vrefresh;
> >
> >       connector = get_connector_by_id(dev, con_id);
> >       if (!connector || !connector->count_modes) @@ -837,16 +842,19 @@
> > 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)) {
> > -                     /* If the vertical refresh frequency is not specified then return
> the
> > -                      * first mode that match with the name. Else, return the mode
> that match
> > -                      * the name and the specified vertical refresh frequency.
> > +                     /* If the vertical refresh frequency is not specified
> > +                      * then return the first mode that match with the name
> > +                      * and update corresponding vrefresh in pipe_arg.
> > +                      * Else, return the mode that match the name and
> > +                      * the specified vertical refresh frequency.
> >                        */
> > -                     mode_vrefresh = mode->clock * 1000.00
> > -                                     / (mode->htotal * mode->vtotal);
> > -                     if (vrefresh == 0)
> > +                     if (*vrefresh == 0) {
> > +                             *vrefresh = get_floating_vrefresh(mode);
> >                               return mode;
> > -                     else if (fabs(mode_vrefresh - vrefresh) < 0.005)
> > +                     } else if (fabs(get_floating_vrefresh(mode)
> > +                                - *vrefresh) < 0.005) {
> >                               return mode;
> > +                     }
> >               }
> >       }
> >
> > @@ -909,11 +917,11 @@ 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->vrefresh);
> > +                                        pipe->mode_str,
> > + &pipe->vrefresh);
> >               if (mode == NULL) {
> >                       fprintf(stderr,
> > -                             "failed to find mode \"%s\" for connector %s\n",
> > -                             pipe->mode_str, pipe->cons[i]);
> > +                     "failed to find mode \"%s-%.2fHz\" for connector %s\n",
> > +                      pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
> 
> We didn't find any mode so having connector_find_mode() update vrefresh
> for us is nonsense.

Yes that's correct, but we need it in a scenario where user provided a vrefresh for the mode and mode is not available with that exact vrefresh but available with a different refresh rate. So for e.g if 1920x1080-60 is only available and user set 1920x1080-59.92 (which is not available). Then it should display that mode 1920x1080-59.92 is not available and not that 1920x1080 is not found (since 1920x1080-60 is available). So I guess best way to handle not found mode is to print vrefresh too only in scenario where user had provided a specific refresh rate as arg. What's your opinion ?


> 
> >                       return -EINVAL;
> >               }
> >       }
> > --
> > 2.7.4
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH libdrm] modetest: Use floating vrefresh while dumping mode
  2019-11-27  7:28   ` Devarsh Thakkar
@ 2019-11-27 12:59     ` Ville Syrjälä
  0 siblings, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2019-11-27 12:59 UTC (permalink / raw)
  To: Devarsh Thakkar
  Cc: Ranganathan Sk, vcu-team, Dhaval Rajeshbhai Shah, dri-devel,
	Varunkumar Allagadapa, Satish Kumar Nagireddy, Rohit Visavalia

On Wed, Nov 27, 2019 at 07:28:31AM +0000, Devarsh Thakkar wrote:
> Thanks for the review Ville, please see inline:
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 26 November 2019 07:15
> > To: Devarsh Thakkar <DEVARSHT@xilinx.com>
> > Cc: dri-devel@lists.freedesktop.org; Hyun Kwon <hyunk@xilinx.com>; vcu-
> > team <vcu-team@xilinx.com>; Ranganathan Sk <rsk@xilinx.com>; Dhaval
> > Rajeshbhai Shah <dshah@xilinx.com>; Satish Kumar Nagireddy
> > <SATISHNA@xilinx.com>; Varunkumar Allagadapa <VARUNKUM@xilinx.com>;
> > Rohit Visavalia <RVISAVAL@xilinx.com>
> > Subject: Re: [PATCH libdrm] modetest: Use floating vrefresh while dumping
> > mode
> > 
> > EXTERNAL EMAIL
> > 
> > On Tue, Nov 26, 2019 at 07:03:58AM -0800, Devarsh Thakkar wrote:
> > > Add inline function to derive floating value of vertical refresh rate
> > > from pixel clock, horizontal total size and vertical total size.
> > >
> > > Use this function to find suitable mode having vrefresh value which is
> > > matching with user provided vrefresh value.
> > >
> > > If user doesn't provide any vrefresh value in args then update
> > > vertical refresh rate value in pipe args using this function so that
> > > it will be used for printing floating vrefresh while dumping mode.
> > >
> > > This will give more accurate picture to user for available modes
> > > differentiated by floating vertical refresh rate and help user select
> > > more appropriate mode using suitable refresh rate value.
> > >
> > > Signed-off-by: Devarsh Thakkar <devarsh.thakkar@xilinx.com>
> > > ---
> > >  tests/modetest/modetest.c | 36 ++++++++++++++++++++++--------------
> > >  1 file changed, 22 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > index b4edfcb..80cf076 100644
> > > --- a/tests/modetest/modetest.c
> > > +++ b/tests/modetest/modetest.c
> > > @@ -133,6 +133,12 @@ static inline int64_t U642I64(uint64_t val)
> > >       return (int64_t)*((int64_t *)&val);  }
> > >
> > > +static inline float get_floating_vrefresh(drmModeModeInfo *mode)
> > 
> > Drop the 'inline'. I'd call it just mode_vrefresh() or somehting like that.
> > 
> 
> Yes I do agree that most compilers will substitute function body if needed (and I do see that in generated assembly even without inline) but I still thought better to add inline keyword just to give the hint to compiler in case if it doesn't.
> Any specific reason or issues if we use inline ?

My usual rule is to use static inline only in header files,
otherwise I never use it unless it shows a tangible performance
benefit.

> 
> 
> > > +{
> > > +     return  mode->clock * 1000.00
> > > +                     / (mode->htotal * mode->vtotal); }
> > > +
> > >  #define bit_name_fn(res)                                     \
> > >  const char * res##_str(int type) {                           \
> > >       unsigned int i;                                         \
> > > @@ -210,9 +216,9 @@ static void dump_encoders(struct device *dev)
> > >
> > >  static void dump_mode(drmModeModeInfo *mode)  {
> > > -     printf("  %s %d %d %d %d %d %d %d %d %d %d",
> > > +     printf("  %s %.2f %d %d %d %d %d %d %d %d %d",
> > >              mode->name,
> > > -            mode->vrefresh,
> > > +            get_floating_vrefresh(mode),
> > >              mode->hdisplay,
> > >              mode->hsync_start,
> > >              mode->hsync_end,
> > > @@ -823,12 +829,11 @@ struct plane_arg {
> > >
> > >  static drmModeModeInfo *
> > >  connector_find_mode(struct device *dev, uint32_t con_id, const char
> > *mode_str,
> > > -     const float vrefresh)
> > > +     float *vrefresh)
> > >  {
> > >       drmModeConnector *connector;
> > >       drmModeModeInfo *mode;
> > >       int i;
> > > -     float mode_vrefresh;
> > >
> > >       connector = get_connector_by_id(dev, con_id);
> > >       if (!connector || !connector->count_modes) @@ -837,16 +842,19 @@
> > > 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)) {
> > > -                     /* If the vertical refresh frequency is not specified then return
> > the
> > > -                      * first mode that match with the name. Else, return the mode
> > that match
> > > -                      * the name and the specified vertical refresh frequency.
> > > +                     /* If the vertical refresh frequency is not specified
> > > +                      * then return the first mode that match with the name
> > > +                      * and update corresponding vrefresh in pipe_arg.
> > > +                      * Else, return the mode that match the name and
> > > +                      * the specified vertical refresh frequency.
> > >                        */
> > > -                     mode_vrefresh = mode->clock * 1000.00
> > > -                                     / (mode->htotal * mode->vtotal);
> > > -                     if (vrefresh == 0)
> > > +                     if (*vrefresh == 0) {
> > > +                             *vrefresh = get_floating_vrefresh(mode);
> > >                               return mode;
> > > -                     else if (fabs(mode_vrefresh - vrefresh) < 0.005)
> > > +                     } else if (fabs(get_floating_vrefresh(mode)
> > > +                                - *vrefresh) < 0.005) {
> > >                               return mode;
> > > +                     }
> > >               }
> > >       }
> > >
> > > @@ -909,11 +917,11 @@ 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->vrefresh);
> > > +                                        pipe->mode_str,
> > > + &pipe->vrefresh);
> > >               if (mode == NULL) {
> > >                       fprintf(stderr,
> > > -                             "failed to find mode \"%s\" for connector %s\n",
> > > -                             pipe->mode_str, pipe->cons[i]);
> > > +                     "failed to find mode \"%s-%.2fHz\" for connector %s\n",
> > > +                      pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
> > 
> > We didn't find any mode so having connector_find_mode() update vrefresh
> > for us is nonsense.
> 
> Yes that's correct, but we need it in a scenario where user provided a vrefresh for the mode and mode is not available with that exact vrefresh but available with a different refresh rate. So for e.g if 1920x1080-60 is only available and user set 1920x1080-59.92 (which is not available). Then it should display that mode 1920x1080-59.92 is not available and not that 1920x1080 is not found (since 1920x1080-60 is available). So I guess best way to handle not found mode is to print vrefresh too only in scenario where user had provided a specific refresh rate as arg. What's your opinion ?

Yeah, only printing it if the user asked for it seems like the sanest
option.

> 
> 
> > 
> > >                       return -EINVAL;
> > >               }
> > >       }
> > > --
> > > 2.7.4
> > 
> > --
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2019-11-27 12:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 15:03 [PATCH libdrm] modetest: Use floating vrefresh while dumping mode Devarsh Thakkar
2019-11-26 15:15 ` Ville Syrjälä
2019-11-27  7:28   ` Devarsh Thakkar
2019-11-27 12:59     ` Ville Syrjälä

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.