| Message ID | 1399319548-16567-2-git-send-email-ajaykumar.rs@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar <ajaykumar.rs@samsung.com> wrote: > As of now, we can have only one bridge hanging off the encoder. > With this patch, we allow multiple bridges to hang onto the same encoder > with the use of a simple next_bridge ptr. > > The drm core calls bridge->funcs for the first bridge which > is attached to it, and its upto the individual bridge drivers > to call bridge->funcs for the next bridge in the chain. > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > --- > drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- > include/drm/drm_crtc.h | 2 ++ > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index d8b7099..fe9905f 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all); > * Zero on success, error code on failure. > */ > int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, > - const struct drm_bridge_funcs *funcs) > + struct drm_encoder *encoder, > + const struct drm_bridge_funcs *funcs) IMO, we should let whoever is spawning the bridges chain them together, instead of passing encoder in init(). Sean > { > + struct drm_bridge *temp; > int ret; > > drm_modeset_lock_all(dev); > @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, > bridge->dev = dev; > bridge->funcs = funcs; > > + if (encoder->bridge) { > + temp = encoder->bridge; > + while (temp->next_bridge) > + temp = temp->next_bridge; > + > + temp->next_bridge = bridge; > + } else > + encoder->bridge = bridge; > + > list_add_tail(&bridge->head, &dev->mode_config.bridge_list); > dev->mode_config.num_bridge++; > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index e55fccb..bb6ed88 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -619,6 +619,7 @@ struct drm_bridge_funcs { > struct drm_bridge { > struct drm_device *dev; > struct list_head head; > + struct drm_bridge *next_bridge; > > struct drm_mode_object base; > > @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector *connector); > extern void drm_connector_unplug_all(struct drm_device *dev); > > extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, > + struct drm_encoder *encoder, > const struct drm_bridge_funcs *funcs); > extern void drm_bridge_cleanup(struct drm_bridge *bridge); > > -- > 1.8.1.2 >
On Tue, May 6, 2014 at 11:55 AM, Sean Paul <seanpaul@chromium.org> wrote: > On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar <ajaykumar.rs@samsung.com> wrote: >> As of now, we can have only one bridge hanging off the encoder. >> With this patch, we allow multiple bridges to hang onto the same encoder >> with the use of a simple next_bridge ptr. >> >> The drm core calls bridge->funcs for the first bridge which >> is attached to it, and its upto the individual bridge drivers >> to call bridge->funcs for the next bridge in the chain. >> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >> --- >> drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- >> include/drm/drm_crtc.h | 2 ++ >> 2 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> index d8b7099..fe9905f 100644 >> --- a/drivers/gpu/drm/drm_crtc.c >> +++ b/drivers/gpu/drm/drm_crtc.c >> @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all); >> * Zero on success, error code on failure. >> */ >> int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >> - const struct drm_bridge_funcs *funcs) >> + struct drm_encoder *encoder, >> + const struct drm_bridge_funcs *funcs) > > > IMO, we should let whoever is spawning the bridges chain them > together, instead of passing encoder in init(). that plus, might be a good time to start adding some static-inline helper fxns for fxn ptr calls like we have for drm_panel.. not a big deal, but I guess it would be a good time to do it now before we start adding chained bridge calls in all the bridges. but yeah, in general this is what I had in mind BR, -R > Sean > > >> { >> + struct drm_bridge *temp; >> int ret; >> >> drm_modeset_lock_all(dev); >> @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >> bridge->dev = dev; >> bridge->funcs = funcs; >> >> + if (encoder->bridge) { >> + temp = encoder->bridge; >> + while (temp->next_bridge) >> + temp = temp->next_bridge; >> + >> + temp->next_bridge = bridge; >> + } else >> + encoder->bridge = bridge; >> + >> list_add_tail(&bridge->head, &dev->mode_config.bridge_list); >> dev->mode_config.num_bridge++; >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> index e55fccb..bb6ed88 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -619,6 +619,7 @@ struct drm_bridge_funcs { >> struct drm_bridge { >> struct drm_device *dev; >> struct list_head head; >> + struct drm_bridge *next_bridge; >> >> struct drm_mode_object base; >> >> @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector *connector); >> extern void drm_connector_unplug_all(struct drm_device *dev); >> >> extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >> + struct drm_encoder *encoder, >> const struct drm_bridge_funcs *funcs); >> extern void drm_bridge_cleanup(struct drm_bridge *bridge); >> >> -- >> 1.8.1.2 >>
Sean, On Tue, May 6, 2014 at 9:25 PM, Sean Paul <seanpaul@chromium.org> wrote: > On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar <ajaykumar.rs@samsung.com> wrote: >> As of now, we can have only one bridge hanging off the encoder. >> With this patch, we allow multiple bridges to hang onto the same encoder >> with the use of a simple next_bridge ptr. >> >> The drm core calls bridge->funcs for the first bridge which >> is attached to it, and its upto the individual bridge drivers >> to call bridge->funcs for the next bridge in the chain. >> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >> --- >> drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- >> include/drm/drm_crtc.h | 2 ++ >> 2 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> index d8b7099..fe9905f 100644 >> --- a/drivers/gpu/drm/drm_crtc.c >> +++ b/drivers/gpu/drm/drm_crtc.c >> @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all); >> * Zero on success, error code on failure. >> */ >> int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >> - const struct drm_bridge_funcs *funcs) >> + struct drm_encoder *encoder, >> + const struct drm_bridge_funcs *funcs) > > > IMO, we should let whoever is spawning the bridges chain them > together, instead of passing encoder in init(). For this, we have to modify drm_bridge_init to return a bridge pointer and then, as Rob is suggesting, we can have some helper functions to update next_bridge ptr to form a bridge chain, and these functions can be called from the corresponding encoder implementation. Ajay >> { >> + struct drm_bridge *temp; >> int ret; >> >> drm_modeset_lock_all(dev); >> @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >> bridge->dev = dev; >> bridge->funcs = funcs; >> >> + if (encoder->bridge) { >> + temp = encoder->bridge; >> + while (temp->next_bridge) >> + temp = temp->next_bridge; >> + >> + temp->next_bridge = bridge; >> + } else >> + encoder->bridge = bridge; >> + >> list_add_tail(&bridge->head, &dev->mode_config.bridge_list); >> dev->mode_config.num_bridge++; >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> index e55fccb..bb6ed88 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -619,6 +619,7 @@ struct drm_bridge_funcs { >> struct drm_bridge { >> struct drm_device *dev; >> struct list_head head; >> + struct drm_bridge *next_bridge; >> >> struct drm_mode_object base; >> >> @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector *connector); >> extern void drm_connector_unplug_all(struct drm_device *dev); >> >> extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >> + struct drm_encoder *encoder, >> const struct drm_bridge_funcs *funcs); >> extern void drm_bridge_cleanup(struct drm_bridge *bridge); >> >> -- >> 1.8.1.2 >>
Rob, On Tue, May 6, 2014 at 9:42 PM, Rob Clark <robdclark@gmail.com> wrote: > On Tue, May 6, 2014 at 11:55 AM, Sean Paul <seanpaul@chromium.org> wrote: >> On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar <ajaykumar.rs@samsung.com> wrote: >>> As of now, we can have only one bridge hanging off the encoder. >>> With this patch, we allow multiple bridges to hang onto the same encoder >>> with the use of a simple next_bridge ptr. >>> >>> The drm core calls bridge->funcs for the first bridge which >>> is attached to it, and its upto the individual bridge drivers >>> to call bridge->funcs for the next bridge in the chain. >>> >>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >>> --- >>> drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- >>> include/drm/drm_crtc.h | 2 ++ >>> 2 files changed, 14 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >>> index d8b7099..fe9905f 100644 >>> --- a/drivers/gpu/drm/drm_crtc.c >>> +++ b/drivers/gpu/drm/drm_crtc.c >>> @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all); >>> * Zero on success, error code on failure. >>> */ >>> int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>> - const struct drm_bridge_funcs *funcs) >>> + struct drm_encoder *encoder, >>> + const struct drm_bridge_funcs *funcs) >> >> >> IMO, we should let whoever is spawning the bridges chain them >> together, instead of passing encoder in init(). > > that plus, might be a good time to start adding some static-inline > helper fxns for fxn ptr calls like we have for drm_panel.. not a big > deal, but I guess it would be a good time to do it now before we start > adding chained bridge calls in all the bridges. Right, I will try to add a few: -- to update next_bridge ptr and form a chain. -- to call pre_enable, enable, disable and post_disable for the next bridge in the list. Ajay > > BR, > -R > >> Sean >> >> >>> { >>> + struct drm_bridge *temp; >>> int ret; >>> >>> drm_modeset_lock_all(dev); >>> @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>> bridge->dev = dev; >>> bridge->funcs = funcs; >>> >>> + if (encoder->bridge) { >>> + temp = encoder->bridge; >>> + while (temp->next_bridge) >>> + temp = temp->next_bridge; >>> + >>> + temp->next_bridge = bridge; >>> + } else >>> + encoder->bridge = bridge; >>> + >>> list_add_tail(&bridge->head, &dev->mode_config.bridge_list); >>> dev->mode_config.num_bridge++; >>> >>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >>> index e55fccb..bb6ed88 100644 >>> --- a/include/drm/drm_crtc.h >>> +++ b/include/drm/drm_crtc.h >>> @@ -619,6 +619,7 @@ struct drm_bridge_funcs { >>> struct drm_bridge { >>> struct drm_device *dev; >>> struct list_head head; >>> + struct drm_bridge *next_bridge; >>> >>> struct drm_mode_object base; >>> >>> @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector *connector); >>> extern void drm_connector_unplug_all(struct drm_device *dev); >>> >>> extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>> + struct drm_encoder *encoder, >>> const struct drm_bridge_funcs *funcs); >>> extern void drm_bridge_cleanup(struct drm_bridge *bridge); >>> >>> -- >>> 1.8.1.2 >>>
On Tue, May 6, 2014 at 12:45 PM, Ajay kumar <ajaynumb@gmail.com> wrote: > Sean, > > > On Tue, May 6, 2014 at 9:25 PM, Sean Paul <seanpaul@chromium.org> wrote: >> On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar <ajaykumar.rs@samsung.com> wrote: >>> As of now, we can have only one bridge hanging off the encoder. >>> With this patch, we allow multiple bridges to hang onto the same encoder >>> with the use of a simple next_bridge ptr. >>> >>> The drm core calls bridge->funcs for the first bridge which >>> is attached to it, and its upto the individual bridge drivers >>> to call bridge->funcs for the next bridge in the chain. >>> >>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >>> --- >>> drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- >>> include/drm/drm_crtc.h | 2 ++ >>> 2 files changed, 14 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >>> index d8b7099..fe9905f 100644 >>> --- a/drivers/gpu/drm/drm_crtc.c >>> +++ b/drivers/gpu/drm/drm_crtc.c >>> @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all); >>> * Zero on success, error code on failure. >>> */ >>> int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>> - const struct drm_bridge_funcs *funcs) >>> + struct drm_encoder *encoder, >>> + const struct drm_bridge_funcs *funcs) >> >> >> IMO, we should let whoever is spawning the bridges chain them >> together, instead of passing encoder in init(). > For this, we have to modify drm_bridge_init to return a bridge pointer Sorry, I don't follow this. You are passing in the bridge pointer, so presumably you already have it. Sean > and > then, as Rob is suggesting, we can have some helper functions to update > next_bridge ptr to form a bridge chain, and these functions can be called from > the corresponding encoder implementation. > > Ajay > >>> { >>> + struct drm_bridge *temp; >>> int ret; >>> >>> drm_modeset_lock_all(dev); >>> @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>> bridge->dev = dev; >>> bridge->funcs = funcs; >>> >>> + if (encoder->bridge) { >>> + temp = encoder->bridge; >>> + while (temp->next_bridge) >>> + temp = temp->next_bridge; >>> + >>> + temp->next_bridge = bridge; >>> + } else >>> + encoder->bridge = bridge; >>> + >>> list_add_tail(&bridge->head, &dev->mode_config.bridge_list); >>> dev->mode_config.num_bridge++; >>> >>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >>> index e55fccb..bb6ed88 100644 >>> --- a/include/drm/drm_crtc.h >>> +++ b/include/drm/drm_crtc.h >>> @@ -619,6 +619,7 @@ struct drm_bridge_funcs { >>> struct drm_bridge { >>> struct drm_device *dev; >>> struct list_head head; >>> + struct drm_bridge *next_bridge; >>> >>> struct drm_mode_object base; >>> >>> @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector *connector); >>> extern void drm_connector_unplug_all(struct drm_device *dev); >>> >>> extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>> + struct drm_encoder *encoder, >>> const struct drm_bridge_funcs *funcs); >>> extern void drm_bridge_cleanup(struct drm_bridge *bridge); >>> >>> -- >>> 1.8.1.2 >>>
On Wed, May 7, 2014 at 1:33 AM, Sean Paul <seanpaul@chromium.org> wrote: > On Tue, May 6, 2014 at 12:45 PM, Ajay kumar <ajaynumb@gmail.com> wrote: >> Sean, >> >> >> On Tue, May 6, 2014 at 9:25 PM, Sean Paul <seanpaul@chromium.org> wrote: >>> On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar <ajaykumar.rs@samsung.com> wrote: >>>> As of now, we can have only one bridge hanging off the encoder. >>>> With this patch, we allow multiple bridges to hang onto the same encoder >>>> with the use of a simple next_bridge ptr. >>>> >>>> The drm core calls bridge->funcs for the first bridge which >>>> is attached to it, and its upto the individual bridge drivers >>>> to call bridge->funcs for the next bridge in the chain. >>>> >>>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >>>> --- >>>> drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- >>>> include/drm/drm_crtc.h | 2 ++ >>>> 2 files changed, 14 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >>>> index d8b7099..fe9905f 100644 >>>> --- a/drivers/gpu/drm/drm_crtc.c >>>> +++ b/drivers/gpu/drm/drm_crtc.c >>>> @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all); >>>> * Zero on success, error code on failure. >>>> */ >>>> int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>>> - const struct drm_bridge_funcs *funcs) >>>> + struct drm_encoder *encoder, >>>> + const struct drm_bridge_funcs *funcs) >>> >>> >>> IMO, we should let whoever is spawning the bridges chain them >>> together, instead of passing encoder in init(). >> For this, we have to modify drm_bridge_init to return a bridge pointer > > Sorry, I don't follow this. You are passing in the bridge pointer, so > presumably you already have it. Ahh, Sorry for that. It should be ptn3460_init, and not drm_bridge_init :) I mean, ptn3460_init shall return a bridge pointer and exynos_dp(encoder) should make use of it to prepare the bridge chain. Ajay > Sean > > > >> and >> then, as Rob is suggesting, we can have some helper functions to update >> next_bridge ptr to form a bridge chain, and these functions can be called from >> the corresponding encoder implementation. >> >> Ajay >> >>>> { >>>> + struct drm_bridge *temp; >>>> int ret; >>>> >>>> drm_modeset_lock_all(dev); >>>> @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>>> bridge->dev = dev; >>>> bridge->funcs = funcs; >>>> >>>> + if (encoder->bridge) { >>>> + temp = encoder->bridge; >>>> + while (temp->next_bridge) >>>> + temp = temp->next_bridge; >>>> + >>>> + temp->next_bridge = bridge; >>>> + } else >>>> + encoder->bridge = bridge; >>>> + >>>> list_add_tail(&bridge->head, &dev->mode_config.bridge_list); >>>> dev->mode_config.num_bridge++; >>>> >>>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >>>> index e55fccb..bb6ed88 100644 >>>> --- a/include/drm/drm_crtc.h >>>> +++ b/include/drm/drm_crtc.h >>>> @@ -619,6 +619,7 @@ struct drm_bridge_funcs { >>>> struct drm_bridge { >>>> struct drm_device *dev; >>>> struct list_head head; >>>> + struct drm_bridge *next_bridge; >>>> >>>> struct drm_mode_object base; >>>> >>>> @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector *connector); >>>> extern void drm_connector_unplug_all(struct drm_device *dev); >>>> >>>> extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, >>>> + struct drm_encoder *encoder, >>>> const struct drm_bridge_funcs *funcs); >>>> extern void drm_bridge_cleanup(struct drm_bridge *bridge); >>>> >>>> -- >>>> 1.8.1.2 >>>>
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d8b7099..fe9905f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -918,8 +918,10 @@ EXPORT_SYMBOL(drm_connector_unplug_all); * Zero on success, error code on failure. */ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, - const struct drm_bridge_funcs *funcs) + struct drm_encoder *encoder, + const struct drm_bridge_funcs *funcs) { + struct drm_bridge *temp; int ret; drm_modeset_lock_all(dev); @@ -931,6 +933,15 @@ int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, bridge->dev = dev; bridge->funcs = funcs; + if (encoder->bridge) { + temp = encoder->bridge; + while (temp->next_bridge) + temp = temp->next_bridge; + + temp->next_bridge = bridge; + } else + encoder->bridge = bridge; + list_add_tail(&bridge->head, &dev->mode_config.bridge_list); dev->mode_config.num_bridge++; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index e55fccb..bb6ed88 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -619,6 +619,7 @@ struct drm_bridge_funcs { struct drm_bridge { struct drm_device *dev; struct list_head head; + struct drm_bridge *next_bridge; struct drm_mode_object base; @@ -862,6 +863,7 @@ extern void drm_connector_cleanup(struct drm_connector *connector); extern void drm_connector_unplug_all(struct drm_device *dev); extern int drm_bridge_init(struct drm_device *dev, struct drm_bridge *bridge, + struct drm_encoder *encoder, const struct drm_bridge_funcs *funcs); extern void drm_bridge_cleanup(struct drm_bridge *bridge);
As of now, we can have only one bridge hanging off the encoder. With this patch, we allow multiple bridges to hang onto the same encoder with the use of a simple next_bridge ptr. The drm core calls bridge->funcs for the first bridge which is attached to it, and its upto the individual bridge drivers to call bridge->funcs for the next bridge in the chain. Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> --- drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- include/drm/drm_crtc.h | 2 ++ 2 files changed, 14 insertions(+), 1 deletion(-)