From: Andrzej Hajda <a.hajda@samsung.com> To: "Jose Abreu" <Jose.Abreu@synopsys.com>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, "Carlos Palminha" <CARLOS.PALMINHA@synopsys.com>, "Alexey Brodkin" <Alexey.Brodkin@synopsys.com>, "Ville Syrjälä" <ville.syrjala@linux.intel.com>, "Dave Airlie" <airlied@linux.ie>, "Archit Taneja" <architt@codeaurora.org> Subject: Re: [PATCH v3 0/6] Introduce new mode validation callbacks Date: Fri, 12 May 2017 15:37:03 +0200 [thread overview] Message-ID: <1b3f1b8a-e3b5-f3e9-4697-bd40bd0cfede@samsung.com> (raw) In-Reply-To: <20170512073232.yxknr556dal6isoh@phenom.ffwll.local> On 12.05.2017 09:32, Daniel Vetter wrote: > On Thu, May 11, 2017 at 10:05:56AM +0100, Jose Abreu wrote: >> This series is a follow up from the discussion at [1]. We start by >> introducing crtc->mode_valid(), encoder->mode_valid() and >> bridge->mode_valid() callbacks which will be used in followup >> patches and also by cleaning the documentation a little bit. >> >> We proceed by introducing new helpers to call this new callbacks >> at 2/6. >> >> At 3/6 a helper function is introduced that calls all mode_valid() >> from a set of bridges. >> >> Next, at 4/6 we modify the connector probe helper so that only modes >> which are supported by a given bridge+encoder+crtc combination are >> probbed. >> >> At 5/6 we call all the mode_valid() callbacks for a given pipeline, >> except the connector->mode_valid one, so that the mode is validated. >> This is done before calling mode_fixup(). >> >> Finally, at 6/6 we use the new crtc->mode_valid() callback in arcpgu >> and remove the atomic_check() callback. >> >> [1] https://patchwork.kernel.org/patch/9702233/ >> >> Jose Abreu (6): >> drm: Add crtc/encoder/bridge->mode_valid() callbacks >> drm: Add drm_{crtc/encoder/connector}_mode_valid() >> drm: Introduce drm_bridge_mode_valid() >> drm: Use new mode_valid() helpers in connector probe helper >> drm: Use mode_valid() in atomic modeset >> drm: arc: Use crtc->mode_valid() callback >> >> Cc: Carlos Palminha <palminha@synopsys.com> >> Cc: Alexey Brodkin <abrodkin@synopsys.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Dave Airlie <airlied@linux.ie> >> Cc: Andrzej Hajda <a.hajda@samsung.com> >> Cc: Archit Taneja <architt@codeaurora.org> > Commented with an entire patch on patch 1, patches 2-5 are all > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > But I think some more acks/r-bs would be really good, since this is quite > a bit change in the helper infrastructure. But otherwise ready for merging > imo. Can you pls also review my proposal for patch 1? > > Thanks, Daniel As the patchset improves many things, I would like to point here that there are still issues with mode probing at least in case of panels. Panels in general does not provide discrete list of supported modes, but they provide list of supported mode ranges (named display_timings), at the moment this is only addressed in drm_panel_funcs::get_timings, but drm core does not use it at all. Currently most of the panel drivers advertises only fixed list of arbitrary chosen modes, and if bridge/connector/encoder/crtc does not support it pipeline does not work, even if slightly different configuration could work. Of course there are workarounds but maybe it would be good to replace drm_connector::modes with drm_connector::timings. This way set of valid timings of the whole pipeline will be intersection of sets of valid timings of every component of the pipeline - quite straightforward and simple construct. As this is just an idea which came to me during patchset review, it is not backed by any code, but maybe it can be interesting for quick brainstorm. Regards Andrzej > >> drivers/gpu/drm/arc/arcpgu_crtc.c | 39 ++++++---- >> drivers/gpu/drm/drm_atomic_helper.c | 76 +++++++++++++++++++- >> drivers/gpu/drm/drm_bridge.c | 33 +++++++++ >> drivers/gpu/drm/drm_crtc_helper_internal.h | 13 ++++ >> drivers/gpu/drm/drm_probe_helper.c | 103 ++++++++++++++++++++++++++- >> include/drm/drm_bridge.h | 22 ++++++ >> include/drm/drm_modeset_helper_vtables.h | 110 ++++++++++++++++++++++------- >> 7 files changed, 348 insertions(+), 48 deletions(-) >> >> -- >> 1.9.1 >> >>
WARNING: multiple messages have this Message-ID (diff)
From: Andrzej Hajda <a.hajda@samsung.com> To: "Jose Abreu" <Jose.Abreu@synopsys.com>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, "Carlos Palminha" <CARLOS.PALMINHA@synopsys.com>, "Alexey Brodkin" <Alexey.Brodkin@synopsys.com>, "Ville Syrjälä" <ville.syrjala@linux.intel.com>, "Dave Airlie" <airlied@linux.ie>, "Archit Taneja" <architt@codeaurora.org> Subject: Re: [PATCH v3 0/6] Introduce new mode validation callbacks Date: Fri, 12 May 2017 15:37:03 +0200 [thread overview] Message-ID: <1b3f1b8a-e3b5-f3e9-4697-bd40bd0cfede@samsung.com> (raw) In-Reply-To: <20170512073232.yxknr556dal6isoh@phenom.ffwll.local> On 12.05.2017 09:32, Daniel Vetter wrote: > On Thu, May 11, 2017 at 10:05:56AM +0100, Jose Abreu wrote: >> This series is a follow up from the discussion at [1]. We start by >> introducing crtc->mode_valid(), encoder->mode_valid() and >> bridge->mode_valid() callbacks which will be used in followup >> patches and also by cleaning the documentation a little bit. >> >> We proceed by introducing new helpers to call this new callbacks >> at 2/6. >> >> At 3/6 a helper function is introduced that calls all mode_valid() >> from a set of bridges. >> >> Next, at 4/6 we modify the connector probe helper so that only modes >> which are supported by a given bridge+encoder+crtc combination are >> probbed. >> >> At 5/6 we call all the mode_valid() callbacks for a given pipeline, >> except the connector->mode_valid one, so that the mode is validated. >> This is done before calling mode_fixup(). >> >> Finally, at 6/6 we use the new crtc->mode_valid() callback in arcpgu >> and remove the atomic_check() callback. >> >> [1] https://patchwork.kernel.org/patch/9702233/ >> >> Jose Abreu (6): >> drm: Add crtc/encoder/bridge->mode_valid() callbacks >> drm: Add drm_{crtc/encoder/connector}_mode_valid() >> drm: Introduce drm_bridge_mode_valid() >> drm: Use new mode_valid() helpers in connector probe helper >> drm: Use mode_valid() in atomic modeset >> drm: arc: Use crtc->mode_valid() callback >> >> Cc: Carlos Palminha <palminha@synopsys.com> >> Cc: Alexey Brodkin <abrodkin@synopsys.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Dave Airlie <airlied@linux.ie> >> Cc: Andrzej Hajda <a.hajda@samsung.com> >> Cc: Archit Taneja <architt@codeaurora.org> > Commented with an entire patch on patch 1, patches 2-5 are all > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > But I think some more acks/r-bs would be really good, since this is quite > a bit change in the helper infrastructure. But otherwise ready for merging > imo. Can you pls also review my proposal for patch 1? > > Thanks, Daniel As the patchset improves many things, I would like to point here that there are still issues with mode probing at least in case of panels. Panels in general does not provide discrete list of supported modes, but they provide list of supported mode ranges (named display_timings), at the moment this is only addressed in drm_panel_funcs::get_timings, but drm core does not use it at all. Currently most of the panel drivers advertises only fixed list of arbitrary chosen modes, and if bridge/connector/encoder/crtc does not support it pipeline does not work, even if slightly different configuration could work. Of course there are workarounds but maybe it would be good to replace drm_connector::modes with drm_connector::timings. This way set of valid timings of the whole pipeline will be intersection of sets of valid timings of every component of the pipeline - quite straightforward and simple construct. As this is just an idea which came to me during patchset review, it is not backed by any code, but maybe it can be interesting for quick brainstorm. Regards Andrzej > >> drivers/gpu/drm/arc/arcpgu_crtc.c | 39 ++++++---- >> drivers/gpu/drm/drm_atomic_helper.c | 76 +++++++++++++++++++- >> drivers/gpu/drm/drm_bridge.c | 33 +++++++++ >> drivers/gpu/drm/drm_crtc_helper_internal.h | 13 ++++ >> drivers/gpu/drm/drm_probe_helper.c | 103 ++++++++++++++++++++++++++- >> include/drm/drm_bridge.h | 22 ++++++ >> include/drm/drm_modeset_helper_vtables.h | 110 ++++++++++++++++++++++------- >> 7 files changed, 348 insertions(+), 48 deletions(-) >> >> -- >> 1.9.1 >> >> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-05-12 13:37 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-05-11 9:05 [PATCH v3 0/6] Introduce new mode validation callbacks Jose Abreu 2017-05-11 9:05 ` [PATCH v3 1/6] drm: Add crtc/encoder/bridge->mode_valid() callbacks Jose Abreu 2017-05-12 7:00 ` Daniel Vetter 2017-05-12 7:00 ` Daniel Vetter 2017-05-12 7:31 ` [PATCH] " Daniel Vetter 2017-05-12 11:29 ` Laurent Pinchart 2017-05-15 6:50 ` Daniel Vetter 2017-05-12 12:37 ` Andrzej Hajda 2017-05-15 6:56 ` Daniel Vetter 2017-05-15 8:10 ` Andrzej Hajda 2017-05-15 8:14 ` Daniel Vetter 2017-05-12 15:59 ` Jose Abreu 2017-05-15 9:11 ` [PATCH 1/2] drm/doc: Document adjusted/request modes a bit better Daniel Vetter 2017-05-15 9:11 ` [PATCH 2/2] drm/doc: Clarify mode_fixup vs. atomic_check a bit more Daniel Vetter 2017-05-16 2:41 ` Jose Abreu 2017-05-16 13:14 ` Andrzej Hajda 2017-05-16 2:38 ` [PATCH 1/2] drm/doc: Document adjusted/request modes a bit better Jose Abreu 2017-05-16 13:17 ` Andrzej Hajda 2017-05-15 9:33 ` [PATCH] drm: Add crtc/encoder/bridge->mode_valid() callbacks Daniel Vetter 2017-05-16 13:15 ` Andrzej Hajda 2017-05-11 9:05 ` [PATCH v3 2/6] drm: Add drm_{crtc/encoder/connector}_mode_valid() Jose Abreu 2017-05-15 8:27 ` Andrzej Hajda 2017-05-15 8:27 ` Andrzej Hajda 2017-05-11 9:05 ` [PATCH v3 3/6] drm: Introduce drm_bridge_mode_valid() Jose Abreu 2017-05-11 9:06 ` [PATCH v3 4/6] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu 2017-05-15 8:39 ` Andrzej Hajda 2017-05-15 8:39 ` Andrzej Hajda 2017-05-15 9:30 ` Daniel Vetter 2017-05-15 9:30 ` Daniel Vetter 2017-05-15 9:51 ` Andrzej Hajda 2017-05-15 9:51 ` Andrzej Hajda 2017-05-11 9:06 ` [PATCH v3 5/6] drm: Use mode_valid() in atomic modeset Jose Abreu 2017-05-15 8:45 ` Andrzej Hajda 2017-05-11 9:06 ` [PATCH v3 6/6] drm: arc: Use crtc->mode_valid() callback Jose Abreu 2017-05-15 8:53 ` Daniel Vetter 2017-05-15 8:53 ` Daniel Vetter 2017-05-15 15:52 ` Daniel Vetter 2017-05-15 15:52 ` Daniel Vetter 2017-05-15 23:55 ` Jose Abreu 2017-05-15 23:55 ` Jose Abreu 2017-05-15 8:54 ` Andrzej Hajda 2017-05-15 8:54 ` Andrzej Hajda 2017-05-12 7:32 ` [PATCH v3 0/6] Introduce new mode validation callbacks Daniel Vetter 2017-05-12 7:32 ` Daniel Vetter 2017-05-12 13:37 ` Andrzej Hajda [this message] 2017-05-12 13:37 ` Andrzej Hajda
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=1b3f1b8a-e3b5-f3e9-4697-bd40bd0cfede@samsung.com \ --to=a.hajda@samsung.com \ --cc=Alexey.Brodkin@synopsys.com \ --cc=CARLOS.PALMINHA@synopsys.com \ --cc=Jose.Abreu@synopsys.com \ --cc=airlied@linux.ie \ --cc=architt@codeaurora.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ --cc=ville.syrjala@linux.intel.com \ /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: linkBe 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.