Hi,

On 27-08-16 20:03, Peter Wu wrote:
On Wed, Aug 24, 2016 at 03:30:11PM +0200, Hans de Goede wrote:
99% of the code in ms_covering_crtc is video-driver agnostic. Add a
screen_is_ms parameter when when FALSE skips the one ms specific check,
this will allow calling ms_covering_crtc on slave GPUs.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/drivers/modesetting/vblank.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/vblank.c 
b/hw/xfree86/drivers/modesetting/vblank.c
index e738497..ec60ac4 100644
--- a/hw/xfree86/drivers/modesetting/vblank.c
+++ b/hw/xfree86/drivers/modesetting/vblank.c
@@ -97,7 +97,7 @@ ms_crtc_on(xf86CrtcPtr crtc)
  */

 static xf86CrtcPtr
-ms_covering_crtc(ScreenPtr pScreen, BoxPtr box)
+ms_covering_crtc(ScreenPtr pScreen, BoxPtr box, Bool screen_is_ms)
 {
     ScrnInfoPtr scrn = xf86ScreenToScrn(pScreen);
     xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -105,14 +105,20 @@ ms_covering_crtc(ScreenPtr pScreen, BoxPtr box)
     int coverage, best_coverage;
     int c;
     BoxRec crtc_box, cover_box;
+    Bool crtc_on;

     best_crtc = NULL;
     best_coverage = 0;
     for (c = 0; c < xf86_config->num_crtc; c++) {
         crtc = xf86_config->crtc[c];

+        if (screen_is_ms)
+            crtc_on = ms_crtc_on(crtc);
+        else
+            crtc_on = crtc->enabled;

This will skip the check whether a screen is on or off via DPMS, right?

Correct, but we do check that the primary fallback crtc is enabled
in patch 3/3 (which introduces the first caller with screen_is_ms = FALSE)
and it is reasonable (if not ideal) to assume that all screens are
on or off at the same time.

If the DPMS property is somehow not valid for output slaves, shouldn't
that be fixed instead?

The dpms state checked by ms_crtc_on is hold in a modesetting driver
private data struct, and the slaves we're calling ms_covering_crtc()
on in patch 3/3 may very well use a different driver and then we
would end up interpreting that driver's private data as if it is
modesetting driver data...

(Please correct/educate me if I am wrong, this is all new for me ;))

No problem, thanks for taking a look at these patches, I hope the
above helps explain things (calling ms_covering_crtc on the crtc-s
of slaves is a bit of a hack, we must be careful to only access
core Xorg / xfree86 data and not driver specific data).

Regards,

Hans




Kind regards,
Peter

+
         /* If the CRTC is off, treat it as not covering */
-        if (!ms_crtc_on(crtc))
+        if (!crtc_on)
             continue;

         ms_crtc_box(crtc, &crtc_box);
@@ -137,7 +143,7 @@ ms_dri2_crtc_covering_drawable(DrawablePtr pDraw)
     box.x2 = box.x1 + pDraw->width;
     box.y2 = box.y1 + pDraw->height;

-    return ms_covering_crtc(pScreen, &box);
+    return ms_covering_crtc(pScreen, &box, TRUE);
 }

 static Bool
--
2.9.3


_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to