From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Archit Taneja <architt@codeaurora.org> Cc: dri-devel@lists.freedesktop.org, Jose Abreu <Jose.Abreu@synopsys.com>, Daniel Vetter <daniel.vetter@ffwll.ch>, Alexey Brodkin <Alexey.Brodkin@synopsys.com>, linux-kernel@vger.kernel.org, Carlos Palminha <CARLOS.PALMINHA@synopsys.com> Subject: Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Date: Fri, 12 May 2017 14:01:49 +0300 [thread overview] Message-ID: <2203839.UYlrXxpDjl@avalon> (raw) In-Reply-To: <ba918745-4e96-aca2-502c-532891d3615c@codeaurora.org> Hi Archit, On Friday 12 May 2017 16:20:07 Archit Taneja wrote: > On 05/12/2017 03:08 PM, Laurent Pinchart wrote: > > On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote: > >> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote: > >>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote: > >>>> Introduce a new helper function which calls mode_valid() callback > >>>> for all bridges in an encoder chain. > >>>> > >>>> Signed-off-by: Jose Abreu <joabreu@synopsys.com> > >>>> 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> > >>>> --- > >>>> > >>>> drivers/gpu/drm/drm_bridge.c | 33 +++++++++++++++++++++++++++++++++ > >>>> include/drm/drm_bridge.h | 2 ++ > >>>> 2 files changed, 35 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_bridge.c > >>>> b/drivers/gpu/drm/drm_bridge.c > >>>> index 86a7637..dc8cdfe 100644 > >>>> --- a/drivers/gpu/drm/drm_bridge.c > >>>> +++ b/drivers/gpu/drm/drm_bridge.c > >>>> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge > >>>> *bridge, > >>>> > >>>> EXPORT_SYMBOL(drm_bridge_mode_fixup); > >>>> > >>>> /** > >>>> > >>>> + * drm_bridge_mode_valid - validate the mode against all bridges in > >>>> the > >>>> + * encoder chain. > >>>> + * @bridge: bridge control structure > >>>> + * @mode: desired mode to be validated > >>>> + * > >>>> + * Calls &drm_bridge_funcs.mode_valid for all the bridges in the > >>>> encoder > >>>> + * chain, starting from the first bridge to the last. If at least one > >>>> bridge + * does not accept the mode the function returns the error > >>>> code. > >>>> + * > >>>> + * Note: the bridge passed should be the one closest to the encoder. > >>>> + * > >>>> + * RETURNS: > >>>> + * MODE_OK on success, drm_mode_status Enum error code on failure > >>>> + */ > >>>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge, > >>>> + const struct drm_display_mode > > > > *mode) > > > >>>> +{ > >>>> + enum drm_mode_status ret = MODE_OK; > >>>> + > >>>> + if (!bridge) > >>>> + return ret; > >>>> + > >>>> + if (bridge->funcs->mode_valid) > >>>> + ret = bridge->funcs->mode_valid(bridge, mode); > >>>> + > >>>> + if (ret != MODE_OK) > >>>> + return ret; > >>>> + > >>>> + return drm_bridge_mode_valid(bridge->next, mode); > >>> > >>> Looks like it should be pretty trivial to avoid the recursion. > >>> > >>> Am I correct in interpreting this that bridges have some kind of > >>> a hand rolled linked list implementation? Reusing the standard > >>> linked lists would allow you to use list_for_each() etc. > >> > >> Yeah it's a hand-rolled list, but current hw also has a bridge nesting > >> depth of 2, so it really doesn't matter. I guess once we have real long > >> chains of bridges we can fix this (and just using list_head sounds like a > >> great idea). > > > > Even if not really needed right now, it's a pretty easy cleanup, if Jose > > has time to handle it in v3 of this series let's not postpone it ;-) > > jfyi, some of the bridge functions call the ops from the last bridge in the > chain to first, so we'd need to use list_for_each_entry_prev() (or something > like that) for them. And now that I think about it, for some of the operations (especially enable/disable) I believe that the bridge should be able to decide whether to call the next/previous bridge first or to configure its hardware first. I can image bridges that need the previous bridge in the chain to provide a valid clock before they get started, as well as bridges that need to be started with the incoming video signal stopped. -- Regards, Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Archit Taneja <architt@codeaurora.org> Cc: Jose Abreu <Jose.Abreu@synopsys.com>, Daniel Vetter <daniel.vetter@ffwll.ch>, Alexey Brodkin <Alexey.Brodkin@synopsys.com>, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Carlos Palminha <CARLOS.PALMINHA@synopsys.com> Subject: Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Date: Fri, 12 May 2017 14:01:49 +0300 [thread overview] Message-ID: <2203839.UYlrXxpDjl@avalon> (raw) In-Reply-To: <ba918745-4e96-aca2-502c-532891d3615c@codeaurora.org> Hi Archit, On Friday 12 May 2017 16:20:07 Archit Taneja wrote: > On 05/12/2017 03:08 PM, Laurent Pinchart wrote: > > On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote: > >> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote: > >>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote: > >>>> Introduce a new helper function which calls mode_valid() callback > >>>> for all bridges in an encoder chain. > >>>> > >>>> Signed-off-by: Jose Abreu <joabreu@synopsys.com> > >>>> 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> > >>>> --- > >>>> > >>>> drivers/gpu/drm/drm_bridge.c | 33 +++++++++++++++++++++++++++++++++ > >>>> include/drm/drm_bridge.h | 2 ++ > >>>> 2 files changed, 35 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_bridge.c > >>>> b/drivers/gpu/drm/drm_bridge.c > >>>> index 86a7637..dc8cdfe 100644 > >>>> --- a/drivers/gpu/drm/drm_bridge.c > >>>> +++ b/drivers/gpu/drm/drm_bridge.c > >>>> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge > >>>> *bridge, > >>>> > >>>> EXPORT_SYMBOL(drm_bridge_mode_fixup); > >>>> > >>>> /** > >>>> > >>>> + * drm_bridge_mode_valid - validate the mode against all bridges in > >>>> the > >>>> + * encoder chain. > >>>> + * @bridge: bridge control structure > >>>> + * @mode: desired mode to be validated > >>>> + * > >>>> + * Calls &drm_bridge_funcs.mode_valid for all the bridges in the > >>>> encoder > >>>> + * chain, starting from the first bridge to the last. If at least one > >>>> bridge + * does not accept the mode the function returns the error > >>>> code. > >>>> + * > >>>> + * Note: the bridge passed should be the one closest to the encoder. > >>>> + * > >>>> + * RETURNS: > >>>> + * MODE_OK on success, drm_mode_status Enum error code on failure > >>>> + */ > >>>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge, > >>>> + const struct drm_display_mode > > > > *mode) > > > >>>> +{ > >>>> + enum drm_mode_status ret = MODE_OK; > >>>> + > >>>> + if (!bridge) > >>>> + return ret; > >>>> + > >>>> + if (bridge->funcs->mode_valid) > >>>> + ret = bridge->funcs->mode_valid(bridge, mode); > >>>> + > >>>> + if (ret != MODE_OK) > >>>> + return ret; > >>>> + > >>>> + return drm_bridge_mode_valid(bridge->next, mode); > >>> > >>> Looks like it should be pretty trivial to avoid the recursion. > >>> > >>> Am I correct in interpreting this that bridges have some kind of > >>> a hand rolled linked list implementation? Reusing the standard > >>> linked lists would allow you to use list_for_each() etc. > >> > >> Yeah it's a hand-rolled list, but current hw also has a bridge nesting > >> depth of 2, so it really doesn't matter. I guess once we have real long > >> chains of bridges we can fix this (and just using list_head sounds like a > >> great idea). > > > > Even if not really needed right now, it's a pretty easy cleanup, if Jose > > has time to handle it in v3 of this series let's not postpone it ;-) > > jfyi, some of the bridge functions call the ops from the last bridge in the > chain to first, so we'd need to use list_for_each_entry_prev() (or something > like that) for them. And now that I think about it, for some of the operations (especially enable/disable) I believe that the bridge should be able to decide whether to call the next/previous bridge first or to configure its hardware first. I can image bridges that need the previous bridge in the chain to provide a valid clock before they get started, as well as bridges that need to be started with the incoming video signal stopped. -- Regards, Laurent Pinchart _______________________________________________ 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 11:01 UTC|newest] Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-05-09 17:00 [PATCH v2 0/8] Introduce new mode validation callbacks Jose Abreu 2017-05-09 17:00 ` [PATCH v2 1/8] drm: Add crtc/encoder/bridge->mode_valid() callbacks Jose Abreu 2017-05-10 8:03 ` Daniel Vetter 2017-05-10 8:03 ` Daniel Vetter 2017-05-10 9:05 ` Jose Abreu 2017-05-12 8:24 ` Laurent Pinchart 2017-05-12 8:24 ` Laurent Pinchart 2017-05-15 6:46 ` Daniel Vetter 2017-05-15 6:46 ` Daniel Vetter 2017-05-09 17:00 ` [PATCH v2 2/8] drm: Add drm_crtc_mode_valid() Jose Abreu 2017-05-10 7:59 ` Daniel Vetter 2017-05-10 7:59 ` Daniel Vetter 2017-05-10 8:57 ` Jose Abreu 2017-05-10 15:12 ` Daniel Vetter 2017-05-10 15:12 ` Daniel Vetter 2017-05-09 17:00 ` [PATCH v2 3/8] drm: Add drm_encoder_mode_valid() Jose Abreu 2017-05-09 17:00 ` [PATCH v2 4/8] drm: Add drm_connector_mode_valid() Jose Abreu 2017-05-09 17:00 ` [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu 2017-05-10 8:01 ` Daniel Vetter 2017-05-10 8:01 ` Daniel Vetter 2017-05-10 8:59 ` Jose Abreu 2017-05-12 9:35 ` Laurent Pinchart 2017-05-12 9:35 ` Laurent Pinchart 2017-05-12 16:06 ` Jose Abreu 2017-05-12 16:06 ` Jose Abreu 2017-05-14 11:04 ` Laurent Pinchart 2017-05-14 11:04 ` Laurent Pinchart 2017-05-15 6:47 ` Daniel Vetter 2017-05-15 6:47 ` Daniel Vetter 2017-05-15 7:05 ` Laurent Pinchart 2017-05-15 7:05 ` Laurent Pinchart 2017-05-16 5:11 ` Jose Abreu 2017-05-16 5:11 ` Jose Abreu 2017-05-09 17:00 ` [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid() Jose Abreu 2017-05-10 13:41 ` Ville Syrjälä 2017-05-10 13:41 ` Ville Syrjälä 2017-05-10 14:01 ` Jose Abreu 2017-05-10 14:01 ` Jose Abreu 2017-05-10 14:07 ` Jose Abreu 2017-05-10 14:07 ` Jose Abreu 2017-05-10 15:14 ` Daniel Vetter 2017-05-10 15:14 ` Daniel Vetter 2017-05-12 9:38 ` Laurent Pinchart 2017-05-12 9:38 ` Laurent Pinchart 2017-05-12 10:50 ` Archit Taneja 2017-05-12 10:50 ` Archit Taneja 2017-05-12 11:01 ` Laurent Pinchart [this message] 2017-05-12 11:01 ` Laurent Pinchart 2017-05-15 4:18 ` Archit Taneja 2017-05-15 4:18 ` Archit Taneja 2017-05-15 6:49 ` Daniel Vetter 2017-05-15 6:49 ` Daniel Vetter 2017-05-09 17:00 ` [PATCH v2 7/8] drm: Use mode_valid() in atomic modeset Jose Abreu 2017-05-10 16:08 ` Archit Taneja 2017-05-10 16:08 ` Archit Taneja 2017-05-10 17:55 ` Daniel Vetter 2017-05-10 17:55 ` Daniel Vetter 2017-05-12 9:53 ` Laurent Pinchart 2017-05-12 9:53 ` Laurent Pinchart 2017-05-15 6:50 ` Daniel Vetter 2017-05-09 17:00 ` [PATCH v2 8/8] drm: arc: Use crtc->mode_valid() callback Jose Abreu 2017-05-12 9:57 ` Laurent Pinchart 2017-05-15 1:40 ` Jose Abreu 2017-05-15 1:40 ` Jose Abreu
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=2203839.UYlrXxpDjl@avalon \ --to=laurent.pinchart@ideasonboard.com \ --cc=Alexey.Brodkin@synopsys.com \ --cc=CARLOS.PALMINHA@synopsys.com \ --cc=Jose.Abreu@synopsys.com \ --cc=architt@codeaurora.org \ --cc=daniel.vetter@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ /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.