Re: [PATCH weston v9 8/9] weston: support clone mode on DRM-frontend

[Pekka Paalanen]

On Thu, 31 May 2018 12:49:14 +0300
Pekka Paalanen  wrote:

> On Wed, 30 May 2018 10:00:57 +
> Marius-cristian Vlad  wrote:
> 

> > One more thing I found that using the following config:
> > 
> > [output]
> > name=HDMI-A-1
> > mode=current
> > same-as=HDMI-A-2
> > 
> > [output]
> > name=HDMI-A-2
> > mode=current
> > 
> > I get ``Output 'HDMI-A-2' enabled with head(s) HDMI-A-1, HDMI-A-2.''
> > 
> > I would've expected  to see ``Output 'HDMI-A-1' enabled with head(s)
> > HDMI-A-1, HDMI-A-2.'' I don't know of this matters or not... the other
> > way around is the same (using in HDMI-A-2 section same-as=HDMI-A-1).   
> 
> Right. The name is usually chosen by the controlling output section.
> The output section with the "same-as" key is a slave section, where all
> other keys are ignored. Following the chain of "same-as" keys one finds
> the controlling output section, identified by not having a "same-as"
> key.
> 
> That's odd that the name didn't change with the opposite configuration.
> I'll have to look into it.

Hi,

I tried it again, and I get what I expected:

If section A has same-as=B, then the output will be named B and the
configuration for both heads is taken from section B.

If section B has same-as=A, then the output will be named A and the
configuration is taken from section A.

Do you think "same-as" is not informative enough on implying which
section will actually apply as the configuration, or do you perhaps
understand it the opposite of what it does? Do you have suggestions?

I intended it to read as: This section/head is the same as that other
named section.


Thanks,
pq


pgp1jyeYKrLDe.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v9 8/9] weston: support clone mode on DRM-frontend

[Pekka Paalanen]

On Wed, 30 May 2018 10:00:57 +
Marius-cristian Vlad  wrote:

> Hi, 
> 
> This no longer applies, due to man/weston-drm changes. 

Hi,

yeah, I should probably rebase and re-send.

> Fixing that I get "atomic: get couldn't commit new state: Invalid
> argument". Does that mean that HW doesn't support CRTC sharing you
> mention in the description? Am I using it properly?

Judging from the below, you are using it properly. Unfortunately the PC
graphics cards do not tend to support shared-CRTC clone mode anymore.
Even when they did, there were only specific combinations of connectors
that could be used. One might have more luck with some embedded boards.
The one I've tested with was an i.MX6 board.

Not having a fallback to independent-CRTC clone mode (needs a damage
tracking re-design) makes the clone mode feature hard to work with.
That combined with Weston's current inability to deal with mode setting
failures gracefully makes it a really bad user experience. I believe
the situation will get better eventually with the use of atomic
modesetting with TEST_ONLY.

> One more thing I found that using the following config:
> 
> [output]
> name=HDMI-A-1
> mode=current
> same-as=HDMI-A-2
> 
> [output]
> name=HDMI-A-2
> mode=current
> 
> I get ``Output 'HDMI-A-2' enabled with head(s) HDMI-A-1, HDMI-A-2.''
> 
> I would've expected  to see ``Output 'HDMI-A-1' enabled with head(s)
> HDMI-A-1, HDMI-A-2.'' I don't know of this matters or not... the other
> way around is the same (using in HDMI-A-2 section same-as=HDMI-A-1). 

Right. The name is usually chosen by the controlling output section.
The output section with the "same-as" key is a slave section, where all
other keys are ignored. Following the chain of "same-as" keys one finds
the controlling output section, identified by not having a "same-as"
key.

That's odd that the name didn't change with the opposite configuration.
I'll have to look into it.

To be honest, I haven't paid too much attention to the output naming,
as long as there is a unique name. In libweston the output name is
arbitrary, and it's just weston that uses a head name as an output name.

> On Thu, 2018-04-19 at 15:09 +0300, Pekka Paalanen wrote:
> > From: Pekka Paalanen 
> > 
> > Add a new output section key "same-as" for configuring clone mode. An
> > output marked "same-as" another output will be configured identically
> > to
> > the other output.
> > 
> > The current implementation supports only CRTC sharing for clone mode.
> > Independent CRTC clone mode cannot be supported until output layout
> > logic is moved from libweston into the frontend and libweston's
> > damage
> > tracking issues stemming from overlapping outputs are solved.
> > 
> > Quite a lot of infrastructure is needed to properly configure clone
> > mode. The implemented logic allows easy addition of independent CRTC
> > clone mode once libweston supports it. The idea is that wet_layoutput
> > is
> > the item to be laid out and all weston_outputs a wet_layoutput
> > contains show exactly the same area of the desktop.
> > 
> > The configuration logic attempts to automatically fall back to
> > creating
> > more weston_outputs when all heads do not work under the same
> > weston_output. For now, the fallback path ends with an error message.
> > 
> > Enabling a weston_output is bit complicated, because one needs to
> > first
> > collect all relevant heads, try to attach them all to the
> > weston_output,
> > and then back up head by head until enabling the weston_output
> > succeeds.
> > A new weston_output is created for the left-over heads and the
> > process
> > is repeated.
> > 
> > CRTC-sharing clone mode is the most efficient clone mode, offering
> > synchronized scanout timings, but it is not always supported by
> > hardware.
> > 
> > v9:
> > - replace weston_compositor_set_heads_changed_cb() with
> >   weston_compositor_add_heads_changed_listener()
> > - remove workaround in simple_head_enable()
> > 
> > v6:
> > - Add man-page note about cms-colord.
> > - Don't create an output just to turn it off.
> > 
> > Fixes: https://emea01.safelinks.protection.outlook.com/?url=https%3A%
> > 2F%2Fphabricator.freedesktop.org%2FT7727=02%7C01%7Cmarius-
> > cristian.vlad%40nxp.com%7C2382a4e34bb74b66b07c08d5a5ee862a%7C686ea1d3
> > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636597366236269669=xoi2xcBR3
> > YD9gBAUidQ%2BmoO6oH0QhX3HIvGbGYqySU0%3D=0

Whoa, that was not in my patch.

> > 
> > Signed-off-by: Pekka Paalanen 
> > Acked-by: Derek Foreman 
> > ---
> >  compositor/main.c  | 492
> > ++---
> >  man/weston-drm.man |  12 ++
> >  2 files changed, 484 insertions(+), 20 deletions(-)

Thanks,
pq


pgpvK9IOxJMuv.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v9 8/9] weston: support clone mode on DRM-frontend

[Marius-cristian Vlad]

Hi, 

This no longer applies, due to man/weston-drm changes. 

Fixing that I get "atomic: get couldn't commit new state: Invalid
argument". Does that mean that HW doesn't support CRTC sharing you
mention in the description? Am I using it properly?

One more thing I found that using the following config:

[output]
name=HDMI-A-1
mode=current
same-as=HDMI-A-2

[output]
name=HDMI-A-2
mode=current

I get ``Output 'HDMI-A-2' enabled with head(s) HDMI-A-1, HDMI-A-2.''

I would've expected  to see ``Output 'HDMI-A-1' enabled with head(s)
HDMI-A-1, HDMI-A-2.'' I don't know of this matters or not... the other
way around is the same (using in HDMI-A-2 section same-as=HDMI-A-1). 



On Thu, 2018-04-19 at 15:09 +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen 
> 
> Add a new output section key "same-as" for configuring clone mode. An
> output marked "same-as" another output will be configured identically
> to
> the other output.
> 
> The current implementation supports only CRTC sharing for clone mode.
> Independent CRTC clone mode cannot be supported until output layout
> logic is moved from libweston into the frontend and libweston's
> damage
> tracking issues stemming from overlapping outputs are solved.
> 
> Quite a lot of infrastructure is needed to properly configure clone
> mode. The implemented logic allows easy addition of independent CRTC
> clone mode once libweston supports it. The idea is that wet_layoutput
> is
> the item to be laid out and all weston_outputs a wet_layoutput
> contains show exactly the same area of the desktop.
> 
> The configuration logic attempts to automatically fall back to
> creating
> more weston_outputs when all heads do not work under the same
> weston_output. For now, the fallback path ends with an error message.
> 
> Enabling a weston_output is bit complicated, because one needs to
> first
> collect all relevant heads, try to attach them all to the
> weston_output,
> and then back up head by head until enabling the weston_output
> succeeds.
> A new weston_output is created for the left-over heads and the
> process
> is repeated.
> 
> CRTC-sharing clone mode is the most efficient clone mode, offering
> synchronized scanout timings, but it is not always supported by
> hardware.
> 
> v9:
> - replace weston_compositor_set_heads_changed_cb() with
>   weston_compositor_add_heads_changed_listener()
> - remove workaround in simple_head_enable()
> 
> v6:
> - Add man-page note about cms-colord.
> - Don't create an output just to turn it off.
> 
> Fixes: https://emea01.safelinks.protection.outlook.com/?url=https%3A%
> 2F%2Fphabricator.freedesktop.org%2FT7727=02%7C01%7Cmarius-
> cristian.vlad%40nxp.com%7C2382a4e34bb74b66b07c08d5a5ee862a%7C686ea1d3
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636597366236269669=xoi2xcBR3
> YD9gBAUidQ%2BmoO6oH0QhX3HIvGbGYqySU0%3D=0
> 
> Signed-off-by: Pekka Paalanen 
> Acked-by: Derek Foreman 
> ---
>  compositor/main.c  | 492
> ++---
>  man/weston-drm.man |  12 ++
>  2 files changed, 484 insertions(+), 20 deletions(-)
> 
> diff --git a/compositor/main.c b/compositor/main.c
> index 85c4d338..fe36e69d 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -70,11 +70,41 @@ struct wet_output_config {
>  };
>  
>  struct wet_compositor;
> +struct wet_layoutput;
>  
>  struct wet_head_tracker {
>   struct wl_listener head_destroy_listener;
>  };
>  
> +/** User data for each weston_output */
> +struct wet_output {
> + struct weston_output *output;
> + struct wl_listener output_destroy_listener;
> + struct wet_layoutput *layoutput;
> + struct wl_list link;/**< in
> wet_layoutput::output_list */
> +};
> +
> +#define MAX_CLONE_HEADS 16
> +
> +struct wet_head_array {
> + struct weston_head *heads[MAX_CLONE_HEADS]; /**<
> heads to add */
> + unsigned n; /**< the number
> of heads */
> +};
> +
> +/** A layout output
> + *
> + * Contains wet_outputs that are all clones (independent CRTCs).
> + * Stores output layout information in the future.
> + */
> +struct wet_layoutput {
> + struct wet_compositor *compositor;
> + struct wl_list compositor_link; /**< in
> wet_compositor::layoutput_list */
> + struct wl_list output_list; /**< wet_output::link */
> + char *name;
> + struct weston_config_section *section;
> + struct wet_head_array add;  /**< tmp: heads to add as
> clones */
> +};
> +
>  struct wet_compositor {
>   struct weston_compositor *compositor;
>   struct weston_config *config;
> @@ -83,6 +113,7 @@ struct wet_compositor {
>   struct wl_listener heads_changed_listener;
>   int (*simple_output_configure)(struct weston_output
> *output);
>   bool init_failed;
> + struct wl_list layoutput_list;  /**<
> wet_layoutput::compositor_link */
>  };
>  
>  static FILE *weston_logfile = NULL;
> @@ -1094,12 +1125,6 @@ simple_head_enable(struct wet_compositor *wet,
> struct weston_head *head)
>   struct 

[PATCH weston v9 8/9] weston: support clone mode on DRM-frontend

[Pekka Paalanen]

From: Pekka Paalanen 

Add a new output section key "same-as" for configuring clone mode. An
output marked "same-as" another output will be configured identically to
the other output.

The current implementation supports only CRTC sharing for clone mode.
Independent CRTC clone mode cannot be supported until output layout
logic is moved from libweston into the frontend and libweston's damage
tracking issues stemming from overlapping outputs are solved.

Quite a lot of infrastructure is needed to properly configure clone
mode. The implemented logic allows easy addition of independent CRTC
clone mode once libweston supports it. The idea is that wet_layoutput is
the item to be laid out and all weston_outputs a wet_layoutput
contains show exactly the same area of the desktop.

The configuration logic attempts to automatically fall back to creating
more weston_outputs when all heads do not work under the same
weston_output. For now, the fallback path ends with an error message.

Enabling a weston_output is bit complicated, because one needs to first
collect all relevant heads, try to attach them all to the weston_output,
and then back up head by head until enabling the weston_output succeeds.
A new weston_output is created for the left-over heads and the process
is repeated.

CRTC-sharing clone mode is the most efficient clone mode, offering
synchronized scanout timings, but it is not always supported by
hardware.

v9:
- replace weston_compositor_set_heads_changed_cb() with
  weston_compositor_add_heads_changed_listener()
- remove workaround in simple_head_enable()

v6:
- Add man-page note about cms-colord.
- Don't create an output just to turn it off.

Fixes: https://phabricator.freedesktop.org/T7727

Signed-off-by: Pekka Paalanen 
Acked-by: Derek Foreman 
---
 compositor/main.c  | 492 ++---
 man/weston-drm.man |  12 ++
 2 files changed, 484 insertions(+), 20 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index 85c4d338..fe36e69d 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -70,11 +70,41 @@ struct wet_output_config {
 };
 
 struct wet_compositor;
+struct wet_layoutput;
 
 struct wet_head_tracker {
struct wl_listener head_destroy_listener;
 };
 
+/** User data for each weston_output */
+struct wet_output {
+   struct weston_output *output;
+   struct wl_listener output_destroy_listener;
+   struct wet_layoutput *layoutput;
+   struct wl_list link;/**< in wet_layoutput::output_list */
+};
+
+#define MAX_CLONE_HEADS 16
+
+struct wet_head_array {
+   struct weston_head *heads[MAX_CLONE_HEADS]; /**< heads to add */
+   unsigned n; /**< the number of heads */
+};
+
+/** A layout output
+ *
+ * Contains wet_outputs that are all clones (independent CRTCs).
+ * Stores output layout information in the future.
+ */
+struct wet_layoutput {
+   struct wet_compositor *compositor;
+   struct wl_list compositor_link; /**< in wet_compositor::layoutput_list 
*/
+   struct wl_list output_list; /**< wet_output::link */
+   char *name;
+   struct weston_config_section *section;
+   struct wet_head_array add;  /**< tmp: heads to add as clones */
+};
+
 struct wet_compositor {
struct weston_compositor *compositor;
struct weston_config *config;
@@ -83,6 +113,7 @@ struct wet_compositor {
struct wl_listener heads_changed_listener;
int (*simple_output_configure)(struct weston_output *output);
bool init_failed;
+   struct wl_list layoutput_list;  /**< wet_layoutput::compositor_link */
 };
 
 static FILE *weston_logfile = NULL;
@@ -1094,12 +1125,6 @@ simple_head_enable(struct wet_compositor *wet, struct 
weston_head *head)
struct weston_output *output;
int ret = 0;
 
-   /* Workaround for repeated DRM backend "off" setting.
-* For any other case, we should not have an attached head that is not
-* enabled. */
-   if (weston_head_get_output(head))
-   return;
-
output = weston_compositor_create_output_with_head(wet->compositor,
   head);
if (!output) {
@@ -1121,10 +1146,6 @@ simple_head_enable(struct wet_compositor *wet, struct 
weston_head *head)
return;
}
 
-   /* Escape hatch for DRM backend "off" setting. */
-   if (ret > 0)
-   return;
-
if (weston_output_enable(output) < 0) {
weston_log("Enabling output \"%s\" failed.\n",
   weston_head_get_name(head));
@@ -1220,32 +1241,29 @@ configure_input_device(struct weston_compositor 
*compositor,
 }
 
 static int
-drm_backend_output_configure(struct weston_output *output)
+drm_backend_output_configure(struct weston_output *output,
+struct