From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org,
Thierry Reding <treding@nvidia.com>,
Thomas Zimmermann <tzimmermann@suse.de>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3 7/7] drm: Allow drivers to leave encoder->possible_crtcs==0
Date: Wed, 18 Mar 2020 18:44:52 +0200 [thread overview]
Message-ID: <20200318164452.GB13686@intel.com> (raw)
In-Reply-To: <20200212090849.GQ2363188@phenom.ffwll.local>
On Wed, Feb 12, 2020 at 10:08:49AM +0100, Daniel Vetter wrote:
> On Wed, Feb 12, 2020 at 10:07:55AM +0100, Daniel Vetter wrote:
> > On Tue, Feb 11, 2020 at 07:14:51PM +0200, Ville Syrjälä wrote:
> > > On Tue, Feb 11, 2020 at 06:05:45PM +0100, Daniel Vetter wrote:
> > > > On Tue, Feb 11, 2020 at 06:22:08PM +0200, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > >
> > > > > Let's simplify life of driver by allowing them to leave
> > > > > encoder->possible_crtcs unset if they have no restrictions
> > > > > in crtc<->encoder linkage. We'll just populate possible_crtcs
> > > > > with the full crtc mask when registering the encoder so that
> > > > > userspace doesn't have to deal with drivers not populating
> > > > > this correctly.
> > > > >
> > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > > We might not actually need/want this, but included it here for
> > > > > future reference if that assumption turns out to be wrong.
> > > >
> > > > I think this one is most definitely needed. _Lots_ of drivers get this
> > > > toally wrong and just leave the value blank. It's encoded as official
> > > > fallback in most userspace compositors.
> > >
> > > OK. It's been a while since I dug around so can't really remmber how
> > > this was being handled. I'll reorder before pushing.
> >
> > Hm otoh having "works with all crtcs" as default is a bit dangerous,
> > whereas the "cannot be cloned" default for possible_clones is perfectly
> > safe.
> >
> > So now I'm kinda not sure whether this is a bright idea, and we shouldn't
> > just eat the cost of fixing up all the various WARNING backtraces your
> > previous patch triggers. I've done a full review and the following look
> > suspect:
> >
> > - tegara/sor.c Strangely it's the only one, the other output drivers do
> > seem to set the possible_crtcs mask to something useful.
>
> Strike that, it sets it using tegra_output_find_possible_crtcs().
>
> I think everything is good and we really don't need this patch here to fix
> up possible_crtcs.
Finally pushed the other patches from the series to drm-misc-next.
Thanks for the reviews.
Should the new possible_{crtcs,clones} WARNs start to trigger for
anyone despite our best efforts, please holler and I'll look into
what needs fixing.
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org,
Thierry Reding <treding@nvidia.com>,
Thomas Zimmermann <tzimmermann@suse.de>,
dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 7/7] drm: Allow drivers to leave encoder->possible_crtcs==0
Date: Wed, 18 Mar 2020 18:44:52 +0200 [thread overview]
Message-ID: <20200318164452.GB13686@intel.com> (raw)
In-Reply-To: <20200212090849.GQ2363188@phenom.ffwll.local>
On Wed, Feb 12, 2020 at 10:08:49AM +0100, Daniel Vetter wrote:
> On Wed, Feb 12, 2020 at 10:07:55AM +0100, Daniel Vetter wrote:
> > On Tue, Feb 11, 2020 at 07:14:51PM +0200, Ville Syrjälä wrote:
> > > On Tue, Feb 11, 2020 at 06:05:45PM +0100, Daniel Vetter wrote:
> > > > On Tue, Feb 11, 2020 at 06:22:08PM +0200, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > >
> > > > > Let's simplify life of driver by allowing them to leave
> > > > > encoder->possible_crtcs unset if they have no restrictions
> > > > > in crtc<->encoder linkage. We'll just populate possible_crtcs
> > > > > with the full crtc mask when registering the encoder so that
> > > > > userspace doesn't have to deal with drivers not populating
> > > > > this correctly.
> > > > >
> > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > > We might not actually need/want this, but included it here for
> > > > > future reference if that assumption turns out to be wrong.
> > > >
> > > > I think this one is most definitely needed. _Lots_ of drivers get this
> > > > toally wrong and just leave the value blank. It's encoded as official
> > > > fallback in most userspace compositors.
> > >
> > > OK. It's been a while since I dug around so can't really remmber how
> > > this was being handled. I'll reorder before pushing.
> >
> > Hm otoh having "works with all crtcs" as default is a bit dangerous,
> > whereas the "cannot be cloned" default for possible_clones is perfectly
> > safe.
> >
> > So now I'm kinda not sure whether this is a bright idea, and we shouldn't
> > just eat the cost of fixing up all the various WARNING backtraces your
> > previous patch triggers. I've done a full review and the following look
> > suspect:
> >
> > - tegara/sor.c Strangely it's the only one, the other output drivers do
> > seem to set the possible_crtcs mask to something useful.
>
> Strike that, it sets it using tegra_output_find_possible_crtcs().
>
> I think everything is good and we really don't need this patch here to fix
> up possible_crtcs.
Finally pushed the other patches from the series to drm-misc-next.
Thanks for the reviews.
Should the new possible_{crtcs,clones} WARNs start to trigger for
anyone despite our best efforts, please holler and I'll look into
what needs fixing.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-03-18 16:44 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-11 16:22 [PATCH v3 0/7] drm: Try to fix encoder possible_clones/crtc Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-11 16:22 ` [PATCH v3 1/7] drm: Include the encoder itself in possible_clones Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-11 16:58 ` Daniel Vetter
2020-02-11 16:58 ` [Intel-gfx] " Daniel Vetter
2020-02-11 16:22 ` [PATCH v3 2/7] drm/gma500: Sanitize possible_clones Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-11 16:22 ` [PATCH v3 3/7] drm/exynos: Use drm_encoder_mask() Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-17 2:27 ` Inki Dae
2020-02-17 2:27 ` [Intel-gfx] " Inki Dae
2020-02-25 0:32 ` Inki Dae
2020-02-11 16:22 ` [PATCH v3 4/7] drm/imx: Remove the bogus possible_clones setup Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-11 16:22 ` [PATCH v3 5/7] drm: Validate encoder->possible_clones Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-11 17:02 ` Daniel Vetter
2020-02-11 17:02 ` [Intel-gfx] " Daniel Vetter
2020-02-11 17:13 ` Ville Syrjälä
2020-02-11 17:13 ` [Intel-gfx] " Ville Syrjälä
2020-02-12 8:56 ` Daniel Vetter
2020-02-12 8:56 ` [Intel-gfx] " Daniel Vetter
2020-02-11 16:22 ` [PATCH v3 6/7] drm: Validate encoder->possible_crtcs Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-11 17:04 ` Daniel Vetter
2020-02-11 17:04 ` [Intel-gfx] " Daniel Vetter
2020-09-06 11:19 ` Jan Kiszka
2020-09-06 11:19 ` Jan Kiszka
2020-09-07 7:14 ` Daniel Vetter
2020-09-07 7:14 ` Daniel Vetter
2020-09-07 7:14 ` [Intel-gfx] " Daniel Vetter
2020-09-10 18:18 ` Deucher, Alexander
2020-09-10 18:18 ` Deucher, Alexander
2020-09-10 18:18 ` [Intel-gfx] " Deucher, Alexander
2020-09-29 9:36 ` Jan Kiszka
2020-09-29 9:36 ` Jan Kiszka
2020-09-29 9:36 ` [Intel-gfx] " Jan Kiszka
2020-09-29 20:04 ` Alex Deucher
2020-09-29 20:04 ` Alex Deucher
2020-09-29 20:04 ` [Intel-gfx] " Alex Deucher
2020-12-03 21:30 ` Alex Deucher
2020-12-03 21:30 ` Alex Deucher
2020-12-03 21:30 ` [Intel-gfx] " Alex Deucher
2020-12-09 13:17 ` Daniel Vetter
2020-12-09 13:17 ` Daniel Vetter
2020-12-09 13:17 ` [Intel-gfx] " Daniel Vetter
2020-12-14 20:26 ` Jan Kiszka
2020-12-14 20:26 ` Jan Kiszka
2020-12-14 20:26 ` [Intel-gfx] " Jan Kiszka
2020-02-11 16:22 ` [PATCH v3 7/7] drm: Allow drivers to leave encoder->possible_crtcs==0 Ville Syrjala
2020-02-11 16:22 ` [Intel-gfx] " Ville Syrjala
2020-02-11 17:05 ` Daniel Vetter
2020-02-11 17:05 ` [Intel-gfx] " Daniel Vetter
2020-02-11 17:14 ` Ville Syrjälä
2020-02-11 17:14 ` [Intel-gfx] " Ville Syrjälä
2020-02-12 9:07 ` Daniel Vetter
2020-02-12 9:07 ` [Intel-gfx] " Daniel Vetter
2020-02-12 9:08 ` Daniel Vetter
2020-02-12 9:08 ` [Intel-gfx] " Daniel Vetter
2020-03-18 16:44 ` Ville Syrjälä [this message]
2020-03-18 16:44 ` Ville Syrjälä
2020-12-03 22:16 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm: Try to fix encoder possible_clones/crtc (rev4) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200318164452.GB13686@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=treding@nvidia.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.