wsi_common_display: Deal with vscan values
diff mbox

Message ID 20180615005701.22266-1-keithp@keithp.com
State New
Headers show

Commit Message

Keith Packard June 15, 2018, 12:57 a.m. UTC
We sorted out what 'vscan' means and are trying to use it correctly.

vscan = 0 is the same as vscan = 1, which is slightly annoying; we use
MAX2(vscan, 1) everywhere.

randr doesn't pass vscan at all, so we set wsi mode vscan = 0.

The doublescan flag doubles the vscan value, so we don't need to deal
with that separately, we can just compare flags normally.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 src/vulkan/wsi/wsi_common_display.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Jason Ekstrand June 15, 2018, 1 a.m. UTC | #1
Looks good to me.  With this properly sprinkled on the appropriate patches,
the entire series is

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

On Thu, Jun 14, 2018 at 5:57 PM, Keith Packard <keithp@keithp.com> wrote:

> We sorted out what 'vscan' means and are trying to use it correctly.
>
> vscan = 0 is the same as vscan = 1, which is slightly annoying; we use
> MAX2(vscan, 1) everywhere.
>
> randr doesn't pass vscan at all, so we set wsi mode vscan = 0.
>
> The doublescan flag doubles the vscan value, so we don't need to deal
> with that separately, we can just compare flags normally.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  src/vulkan/wsi/wsi_common_display.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common_display.c
> b/src/vulkan/wsi/wsi_common_display.c
> index c7f794a0eff..de1c1826bd2 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -149,7 +149,7 @@ wsi_display_mode_matches_drm(wsi_display_mode *wsi,
>        wsi->vsync_start == drm->vsync_start &&
>        wsi->vsync_end == drm->vsync_end &&
>        wsi->vtotal == drm->vtotal &&
> -      wsi->vscan == drm->vscan &&
> +      MAX2(wsi->vscan, 1) == MAX2(drm->vscan, 1) &&
>        wsi->flags == drm->flags;
>  }
>
> @@ -158,7 +158,7 @@ wsi_display_mode_refresh(struct wsi_display_mode *wsi)
>  {
>     return (double) wsi->clock * 1000.0 / ((double) wsi->htotal *
>                                            (double) wsi->vtotal *
> -                                          (double) (wsi->vscan + 1));
> +                                          (double) MAX2(wsi->vscan, 1));
>  }
>
>  static uint64_t wsi_get_current_monotonic(void)
> @@ -1657,6 +1657,7 @@ wsi_display_mode_matches_x(struct wsi_display_mode
> *wsi,
>        wsi->vsync_start == xcb->vsync_start &&
>        wsi->vsync_end == xcb->vsync_end &&
>        wsi->vtotal == xcb->vtotal &&
> +      wsi->vscan <= 1 &&
>        wsi->flags == xcb->mode_flags;
>  }
>
> @@ -1707,8 +1708,6 @@ wsi_display_register_x_mode(struct wsi_device
> *wsi_device,
>     display_mode->vsync_end = x_mode->vsync_end;
>     display_mode->vtotal = x_mode->vtotal;
>     display_mode->vscan = 0;
> -   if (x_mode->mode_flags & XCB_RANDR_MODE_FLAG_DOUBLE_SCAN)
> -      display_mode->vscan = 1;
>     display_mode->flags = x_mode->mode_flags;
>
>     list_addtail(&display_mode->list, &connector->display_modes);
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Keith Packard June 15, 2018, 1:37 a.m. UTC | #2
Jason Ekstrand <jason@jlekstrand.net> writes:

> Looks good to me.  With this properly sprinkled on the appropriate patches,
> the entire series is
>
> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

Thanks so much! I've rebased the series onto current master and pushed
it back to my gitlab repo here

        https://gitlab.freedesktop.org/keithp/mesa/tree/drm-lease

I'll be doing testing tomorrow morning, and if it all looks good on both
anv and radv, I'll get it merged and pushed to master.

Now to get the next series ready for review :-)
Ville Syrjala June 15, 2018, 12:16 p.m. UTC | #3
On Thu, Jun 14, 2018 at 05:57:01PM -0700, Keith Packard wrote:
> We sorted out what 'vscan' means and are trying to use it correctly.
> 
> vscan = 0 is the same as vscan = 1, which is slightly annoying; we use
> MAX2(vscan, 1) everywhere.
> 
> randr doesn't pass vscan at all, so we set wsi mode vscan = 0.
> 
> The doublescan flag doubles the vscan value, so we don't need to deal
> with that separately, we can just compare flags normally.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  src/vulkan/wsi/wsi_common_display.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c
> index c7f794a0eff..de1c1826bd2 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -149,7 +149,7 @@ wsi_display_mode_matches_drm(wsi_display_mode *wsi,
>        wsi->vsync_start == drm->vsync_start &&
>        wsi->vsync_end == drm->vsync_end &&
>        wsi->vtotal == drm->vtotal &&
> -      wsi->vscan == drm->vscan &&
> +      MAX2(wsi->vscan, 1) == MAX2(drm->vscan, 1) &&
>        wsi->flags == drm->flags;
>  }
>  
> @@ -158,7 +158,7 @@ wsi_display_mode_refresh(struct wsi_display_mode *wsi)
>  {
>     return (double) wsi->clock * 1000.0 / ((double) wsi->htotal *
>                                            (double) wsi->vtotal *
> -                                          (double) (wsi->vscan + 1));
> +                                          (double) MAX2(wsi->vscan, 1));

Are you dealing with INTERLACE anywhere? The kernel generally operates
under the assumption that vrefresh == field rate.

>  }
>  
>  static uint64_t wsi_get_current_monotonic(void)
> @@ -1657,6 +1657,7 @@ wsi_display_mode_matches_x(struct wsi_display_mode *wsi,
>        wsi->vsync_start == xcb->vsync_start &&
>        wsi->vsync_end == xcb->vsync_end &&
>        wsi->vtotal == xcb->vtotal &&
> +      wsi->vscan <= 1 && 
>        wsi->flags == xcb->mode_flags;
>  }
>  
> @@ -1707,8 +1708,6 @@ wsi_display_register_x_mode(struct wsi_device *wsi_device,
>     display_mode->vsync_end = x_mode->vsync_end;
>     display_mode->vtotal = x_mode->vtotal;
>     display_mode->vscan = 0;
> -   if (x_mode->mode_flags & XCB_RANDR_MODE_FLAG_DOUBLE_SCAN)
> -      display_mode->vscan = 1;
>     display_mode->flags = x_mode->mode_flags;
>  
>     list_addtail(&display_mode->list, &connector->display_modes);
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Patch
diff mbox

diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c
index c7f794a0eff..de1c1826bd2 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -149,7 +149,7 @@  wsi_display_mode_matches_drm(wsi_display_mode *wsi,
       wsi->vsync_start == drm->vsync_start &&
       wsi->vsync_end == drm->vsync_end &&
       wsi->vtotal == drm->vtotal &&
-      wsi->vscan == drm->vscan &&
+      MAX2(wsi->vscan, 1) == MAX2(drm->vscan, 1) &&
       wsi->flags == drm->flags;
 }
 
@@ -158,7 +158,7 @@  wsi_display_mode_refresh(struct wsi_display_mode *wsi)
 {
    return (double) wsi->clock * 1000.0 / ((double) wsi->htotal *
                                           (double) wsi->vtotal *
-                                          (double) (wsi->vscan + 1));
+                                          (double) MAX2(wsi->vscan, 1));
 }
 
 static uint64_t wsi_get_current_monotonic(void)
@@ -1657,6 +1657,7 @@  wsi_display_mode_matches_x(struct wsi_display_mode *wsi,
       wsi->vsync_start == xcb->vsync_start &&
       wsi->vsync_end == xcb->vsync_end &&
       wsi->vtotal == xcb->vtotal &&
+      wsi->vscan <= 1 && 
       wsi->flags == xcb->mode_flags;
 }
 
@@ -1707,8 +1708,6 @@  wsi_display_register_x_mode(struct wsi_device *wsi_device,
    display_mode->vsync_end = x_mode->vsync_end;
    display_mode->vtotal = x_mode->vtotal;
    display_mode->vscan = 0;
-   if (x_mode->mode_flags & XCB_RANDR_MODE_FLAG_DOUBLE_SCAN)
-      display_mode->vscan = 1;
    display_mode->flags = x_mode->mode_flags;
 
    list_addtail(&display_mode->list, &connector->display_modes);