[PATCH v3] ui/cocoa: Take refresh rate into account

Akihiko Odaki posted 1 patch 1 year, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220620202921.21062-1-akihiko.odaki@gmail.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Akihiko Odaki <akihiko.odaki@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
meson.build |  3 ++-
ui/cocoa.m  | 12 ++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
[PATCH v3] ui/cocoa: Take refresh rate into account
Posted by Akihiko Odaki 1 year, 10 months ago
Retreieve the refresh rate of the display and reflect it with
dpy_set_ui_info() and update_displaychangelistener(), allowing the
guest and DisplayChangeListener to consume the information.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 meson.build |  3 ++-
 ui/cocoa.m  | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 0c2e11ff071..0f83f3730af 100644
--- a/meson.build
+++ b/meson.build
@@ -575,7 +575,8 @@ if get_option('attr').allowed()
   endif
 endif
 
-cocoa = dependency('appleframeworks', modules: 'Cocoa', required: get_option('cocoa'))
+cocoa = dependency('appleframeworks', modules: ['Cocoa', 'CoreVideo'],
+                   required: get_option('cocoa'))
 if cocoa.found() and get_option('sdl').enabled()
   error('Cocoa and SDL cannot be enabled at the same time')
 endif
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 84c84e98fc5..0000a3949c6 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -561,8 +561,20 @@ - (void) updateUIInfoLocked
         CGDirectDisplayID display = [[description objectForKey:@"NSScreenNumber"] unsignedIntValue];
         NSSize screenSize = [[[self window] screen] frame].size;
         CGSize screenPhysicalSize = CGDisplayScreenSize(display);
+        CVDisplayLinkRef displayLink;
 
         frameSize = isFullscreen ? screenSize : [self frame].size;
+
+        if (!CVDisplayLinkCreateWithCGDisplay(display, &displayLink)) {
+            CVTime period = CVDisplayLinkGetNominalOutputVideoRefreshPeriod(displayLink);
+            CVDisplayLinkRelease(displayLink);
+            if (!(period.flags & kCVTimeIsIndefinite)) {
+                update_displaychangelistener(&dcl,
+                                             1000 * period.timeValue / period.timeScale);
+                info.refresh_rate = (int64_t)1000 * period.timeScale / period.timeValue;
+            }
+        }
+
         info.width_mm = frameSize.width / screenSize.width * screenPhysicalSize.width;
         info.height_mm = frameSize.height / screenSize.height * screenPhysicalSize.height;
     } else {
-- 
2.32.1 (Apple Git-133)
Re: [PATCH v3] ui/cocoa: Take refresh rate into account
Posted by Peter Maydell 1 year, 10 months ago
On Mon, 20 Jun 2022 at 21:29, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> Retreieve the refresh rate of the display and reflect it with
> dpy_set_ui_info() and update_displaychangelistener(), allowing the
> guest and DisplayChangeListener to consume the information.

But why? What goes wrong if we don't bother to do this?

thanks
-- PMM
Re: [PATCH v3] ui/cocoa: Take refresh rate into account
Posted by Gerd Hoffmann 1 year, 10 months ago
On Tue, Jun 21, 2022 at 09:51:38AM +0100, Peter Maydell wrote:
> On Mon, 20 Jun 2022 at 21:29, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> >
> > Retreieve the refresh rate of the display and reflect it with
> > dpy_set_ui_info() and update_displaychangelistener(), allowing the
> > guest and DisplayChangeListener to consume the information.
> 
> But why? What goes wrong if we don't bother to do this?

Nothing goes wrong.  This provides a hint to the guest how often the
display is updated, so the guest has the chance to adapt to that.
When we run 30 Hz display updates on the host side it is pointless for
the guest to update the screen at 60Hz frequency, the guest can spare
some cpu cycles instead.

[ this should be better explained in the commit message ]

take care,
  Gerd
Re: [PATCH v3] ui/cocoa: Take refresh rate into account
Posted by Peter Maydell 1 year, 10 months ago
On Fri, 1 Jul 2022 at 11:11, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Tue, Jun 21, 2022 at 09:51:38AM +0100, Peter Maydell wrote:
> > On Mon, 20 Jun 2022 at 21:29, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > >
> > > Retreieve the refresh rate of the display and reflect it with
> > > dpy_set_ui_info() and update_displaychangelistener(), allowing the
> > > guest and DisplayChangeListener to consume the information.
> >
> > But why? What goes wrong if we don't bother to do this?
>
> Nothing goes wrong.  This provides a hint to the guest how often the
> display is updated, so the guest has the chance to adapt to that.
> When we run 30 Hz display updates on the host side it is pointless for
> the guest to update the screen at 60Hz frequency, the guest can spare
> some cpu cycles instead.
>
> [ this should be better explained in the commit message ]

Thanks for the explanation.

-- PMM
Re: [PATCH v3] ui/cocoa: Take refresh rate into account
Posted by Akihiko Odaki 1 year, 10 months ago
On 2022/06/21 17:51, Peter Maydell wrote:
> On Mon, 20 Jun 2022 at 21:29, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>>
>> Retreieve the refresh rate of the display and reflect it with
>> dpy_set_ui_info() and update_displaychangelistener(), allowing the
>> guest and DisplayChangeListener to consume the information.
> 
> But why? What goes wrong if we don't bother to do this?
> 
> thanks
> -- PMM

Regarding dpy_set_ui_info(), it depends on the guest. 
update_displaychangelistener() would change the frequency of the calls 
of the DisplayChangeListener.

I think it is obvious that delivering the refresh rate with 
dpy_set_ui_info() and update_displaychangelistener() makes sense, 
considering that those functions actually exist. They shouldn't exist at 
first place if we don't have to bother to do this.

Regards,
Akihiko Odaki