On Thu, 12 Apr 2018 12:09:26 +0200
Daniel Stone <dan...@fooishbar.org> wrote:

> Hi Pekka,
> 
> On 16 February 2018 at 15:56, Pekka Paalanen <ppaala...@gmail.com> wrote:
> > here is the v6 of the shared-CRTC clone mode series. Since v5, quite
> > many patches have been extracted from this series, sent out and merged
> > upstream. However, now the series is bigger than ever, because here I am
> > posting the complete series, including the full DRM-backend migration
> > and DRM shared-CRTC clone mode implementation, thanks to having the
> > basic atomic modesetting landed upstream.  
> 
> I reviewed clonemode-7-part1 from
> https://gitlab.collabora.com/pq/weston, which is slightly revised (&
> significantly truncated) compared to this series. Here's a braindump:
> 
> When adding head_list to a weston_output, we can't send
> wp_presentation_feedback's sync_output event to the same resource for
> multiple wl_outputs:
> https://gitlab.collabora.com/pq/weston/commit/d0f5afff9213b81c329a98cdc7593a0101f953d5#note_7491

Fixed.

> Should be a pretty easy fix (don't iterate through multiple heads once
> you've sent an event to at least one head's resource), but I don't
> know what the long-term fix is. Maybe there isn't a good one.

I think the fix there is very much sufficient.

> I think this is also a small and easily-fixable bug:
> https://gitlab.collabora.com/pq/weston/commit/55206c5e8453722caed18d7fc6e3d9ed0966e623#note_7496

Unfortunately, it's complicated and not a bug. :-P
Explanation in Gitlab.

> The only real interface change I'd like to see is using a listener
> list rather than a single callback:
> https://gitlab.collabora.com/pq/weston/commit/ebb74da3c89d90810f8209340da33faeea3169bd#note_7493

Done.

> Here are a couple of throwaway comments and nitpicks; feel free to ignore 
> them:
> https://gitlab.collabora.com/pq/weston/commit/199fda53546128afb6c0553749dfa85bd00799a4#note_7492
> https://gitlab.collabora.com/pq/weston/commit/c618aa3dc9c2046e5540bbe444375f79e2e2c71f#note_7494
> https://gitlab.collabora.com/pq/weston/commit/6a20dd4e16ae75431149c19e42b52eaf2e179b8c#note_7495

Yeah.

> The rest looks good to me! I think it'd be good to merge 'libweston:
> cancel idle_repaint on output destroy' ASAP in particular. The fbdev
> and RDP patches are quite lazily Acked-by me, as I lost the will to
> live whilst trying to review them. The rest of the series, with at
> least the first three addressed, is Reviewed-by me.

Your R-b and A-b should be in place, the revised series is at
https://gitlab.collabora.com/pq/weston/commits/clonemode-8-part1

I ran through my test procedure from the v6 cover-letter, and
everything worked as expected there, except for the DRM-backend tests.

drmModeAtomicCommit() fails with -EBUSY after the "start with two
outputs, unplug and re-plug one" test. It happens already on upstream
master, so it's not introduced by this series. The panel has seconds
showing, and I am unplugging and replugging a DisplayPort monitor.

The zero outputs test fails on hotplug which looks like my bad in this
series, so I need to fix that before we land this. The test works fine
on upstream master.


Thanks,
pq

Attachment: pgpqKKbYo3WTt.pgp
Description: OpenPGP digital signature

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

Reply via email to