Re: [PATCH weston v6 69/73] compositor-drm: head attach requires a modeset
Hi, On 18 April 2018 at 13:37, Pekka Paalanen wrote: > On Wed, 18 Apr 2018 12:09:45 +0300 Pekka Paalanen wrote: >> Sure, I'll see about adding that. > > looks like I would get into a bit of pickle in > drm_pending_state_apply_atomic() if struct drm_output has heads_dirty > flag. > > Currently the loop over heads is inside state_invalid==true branch, and > I would need to take it out of there if I no longer set state_invalid. > But when I go through the heads, I would not know which output they > were removed from, so I cannot check the output->heads_dirty flag. > Seems like I would need to add pending_removal flag to drm_head or add > a list of removed connectors to the drm_output. Oh, right. I can't think of a good way to get around that as it stands. I think the best thing for now would be to leave the XXX in there, and we can revisit it when the dust has settled, but for now just setting state_invalid is fine. Thanks for following up! Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v6 69/73] compositor-drm: head attach requires a modeset
On Wed, 18 Apr 2018 12:09:45 +0300
Pekka Paalanen wrote:
> On Thu, 12 Apr 2018 14:55:01 +0200
> Daniel Stone wrote:
>
> > On 16 February 2018 at 15:57, Pekka Paalanen wrote:
> > > For the attach on an enabled output to have an effect, we need to go
> > > through drmModeSetCrtc or ATOMIC_ALLOW_MODESET.
> > >
> > > Signed-off-by: Pekka Paalanen
> > > ---
> > > libweston/compositor-drm.c | 15 +++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> > > index d1fc66ac..d0196715 100644
> > > --- a/libweston/compositor-drm.c
> > > +++ b/libweston/compositor-drm.c
> > > @@ -4404,9 +4404,24 @@ static int
> > > drm_output_attach_head(struct weston_output *output_base,
> > >struct weston_head *head_base)
> > > {
> > > + struct drm_backend *b = to_drm_backend(output_base->compositor);
> > > +
> > > if (wl_list_length(&output_base->head_list) >=
> > > MAX_CLONED_CONNECTORS)
> > > return -1;
> > >
> > > + if (!output_base->enabled)
> > > + return 0;
> > > +
> > > + /* XXX: ensure the configuration will work.
> > > +* This is actually impossible without major infrastructure
> > > +* work. */
> > > +
> > > + /* Need to go through modeset to add connectors. */
> > > + /* XXX: Ideally we'd do this per-output, not globally. */
> > > + b->state_invalid = true;
> >
> > How about just an output->heads_dirty flag or something, which would
> > ensure ALLOW_MODESET/SetCrtc are called?
>
> Sure, I'll see about adding that.
Hi Daniel,
looks like I would get into a bit of pickle in
drm_pending_state_apply_atomic() if struct drm_output has heads_dirty
flag.
Currently the loop over heads is inside state_invalid==true branch, and
I would need to take it out of there if I no longer set state_invalid.
But when I go through the heads, I would not know which output they
were removed from, so I cannot check the output->heads_dirty flag.
Seems like I would need to add pending_removal flag to drm_head or add
a list of removed connectors to the drm_output.
Doesn't seem worth it to me, so maybe I'll just remove the XXX comments
and let it be?
Thanks,
pq
pgpSuG5KVi_aC.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 v6 69/73] compositor-drm: head attach requires a modeset
On Thu, 12 Apr 2018 14:55:01 +0200
Daniel Stone wrote:
> On 16 February 2018 at 15:57, Pekka Paalanen wrote:
> > For the attach on an enabled output to have an effect, we need to go
> > through drmModeSetCrtc or ATOMIC_ALLOW_MODESET.
> >
> > Signed-off-by: Pekka Paalanen
> > ---
> > libweston/compositor-drm.c | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> > index d1fc66ac..d0196715 100644
> > --- a/libweston/compositor-drm.c
> > +++ b/libweston/compositor-drm.c
> > @@ -4404,9 +4404,24 @@ static int
> > drm_output_attach_head(struct weston_output *output_base,
> >struct weston_head *head_base)
> > {
> > + struct drm_backend *b = to_drm_backend(output_base->compositor);
> > +
> > if (wl_list_length(&output_base->head_list) >=
> > MAX_CLONED_CONNECTORS)
> > return -1;
> >
> > + if (!output_base->enabled)
> > + return 0;
> > +
> > + /* XXX: ensure the configuration will work.
> > +* This is actually impossible without major infrastructure
> > +* work. */
> > +
> > + /* Need to go through modeset to add connectors. */
> > + /* XXX: Ideally we'd do this per-output, not globally. */
> > + b->state_invalid = true;
>
> How about just an output->heads_dirty flag or something, which would
> ensure ALLOW_MODESET/SetCrtc are called?
Sure, I'll see about adding that.
Thanks,
pq
pgp3byLMj4SOW.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 v6 69/73] compositor-drm: head attach requires a modeset
On 16 February 2018 at 15:57, Pekka Paalanen wrote:
> For the attach on an enabled output to have an effect, we need to go
> through drmModeSetCrtc or ATOMIC_ALLOW_MODESET.
>
> Signed-off-by: Pekka Paalanen
> ---
> libweston/compositor-drm.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index d1fc66ac..d0196715 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -4404,9 +4404,24 @@ static int
> drm_output_attach_head(struct weston_output *output_base,
>struct weston_head *head_base)
> {
> + struct drm_backend *b = to_drm_backend(output_base->compositor);
> +
> if (wl_list_length(&output_base->head_list) >= MAX_CLONED_CONNECTORS)
> return -1;
>
> + if (!output_base->enabled)
> + return 0;
> +
> + /* XXX: ensure the configuration will work.
> +* This is actually impossible without major infrastructure
> +* work. */
> +
> + /* Need to go through modeset to add connectors. */
> + /* XXX: Ideally we'd do this per-output, not globally. */
> + b->state_invalid = true;
How about just an output->heads_dirty flag or something, which would
ensure ALLOW_MODESET/SetCrtc are called?
Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
