diff mbox series

[4/4] drm/bridge: ti-sn65dsi86: use helper to lookup panel-id

Message ID 20190630203614.5290-5-robdclark@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show
Series drm+dt+efi: support devices with multiple possible panels | expand

Commit Message

Rob Clark June 30, 2019, 8:36 p.m. UTC 
From: Rob Clark <robdclark@chromium.org>

Use the drm_of_find_panel_id() helper to decide which endpoint to use
when looking up panel.  This way we can support devices that have
multiple possible panels, such as the aarch64 laptops.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart June 30, 2019, 9:17 p.m. UTC | #1 
Hi Rob,

Thank you for the patch.

On Sun, Jun 30, 2019 at 01:36:08PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Use the drm_of_find_panel_id() helper to decide which endpoint to use
> when looking up panel.  This way we can support devices that have
> multiple possible panels, such as the aarch64 laptops.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 2719d9c0864b..56c66a43f1a6 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -790,7 +790,7 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
>  			      const struct i2c_device_id *id)
>  {
>  	struct ti_sn_bridge *pdata;
> -	int ret;
> +	int ret, panel_id;
>  
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>  		DRM_ERROR("device doesn't support I2C\n");
> @@ -811,7 +811,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
>  
>  	pdata->dev = &client->dev;
>  
> -	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
> +	panel_id = drm_of_find_panel_id();
> +	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, panel_id,
>  					  &pdata->panel, NULL);
>  	if (ret) {
>  		DRM_ERROR("could not find any panel node\n");

No, I'm sorry, but that's a no-go. We can't patch every single bridge
driver to support this hack. We need a solution implemented at another
level that will not spread throughout the whole subsystem.
Rob Clark June 30, 2019, 9:50 p.m. UTC | #2 
On Sun, Jun 30, 2019 at 2:17 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> Thank you for the patch.
>
> On Sun, Jun 30, 2019 at 01:36:08PM -0700, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Use the drm_of_find_panel_id() helper to decide which endpoint to use
> > when looking up panel.  This way we can support devices that have
> > multiple possible panels, such as the aarch64 laptops.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index 2719d9c0864b..56c66a43f1a6 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -790,7 +790,7 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> >                             const struct i2c_device_id *id)
> >  {
> >       struct ti_sn_bridge *pdata;
> > -     int ret;
> > +     int ret, panel_id;
> >
> >       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> >               DRM_ERROR("device doesn't support I2C\n");
> > @@ -811,7 +811,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> >
> >       pdata->dev = &client->dev;
> >
> > -     ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
> > +     panel_id = drm_of_find_panel_id();
> > +     ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, panel_id,
> >                                         &pdata->panel, NULL);
> >       if (ret) {
> >               DRM_ERROR("could not find any panel node\n");
>
> No, I'm sorry, but that's a no-go. We can't patch every single bridge
> driver to support this hack. We need a solution implemented at another
> level that will not spread throughout the whole subsystem.
>

it could be possible to make a better helper.. but really there aren't
*that* many bridge drivers

suggestions ofc welcome, but I think one way or another we are going
to need to patch bridges by the time we get to adding ACPI support, so
really trivial couple line patches to the handful of bridges we have
isn't really something that worries me

BR,
-R
Laurent Pinchart June 30, 2019, 9:57 p.m. UTC | #3 
Hi Rob,

On Sun, Jun 30, 2019 at 02:50:59PM -0700, Rob Clark wrote:
> On Sun, Jun 30, 2019 at 2:17 PM Laurent Pinchart wrote:
> > On Sun, Jun 30, 2019 at 01:36:08PM -0700, Rob Clark wrote:
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > Use the drm_of_find_panel_id() helper to decide which endpoint to use
> > > when looking up panel.  This way we can support devices that have
> > > multiple possible panels, such as the aarch64 laptops.
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > ---
> > >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > index 2719d9c0864b..56c66a43f1a6 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > @@ -790,7 +790,7 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> > >                             const struct i2c_device_id *id)
> > >  {
> > >       struct ti_sn_bridge *pdata;
> > > -     int ret;
> > > +     int ret, panel_id;
> > >
> > >       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> > >               DRM_ERROR("device doesn't support I2C\n");
> > > @@ -811,7 +811,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> > >
> > >       pdata->dev = &client->dev;
> > >
> > > -     ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
> > > +     panel_id = drm_of_find_panel_id();
> > > +     ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, panel_id,
> > >                                         &pdata->panel, NULL);
> > >       if (ret) {
> > >               DRM_ERROR("could not find any panel node\n");
> >
> > No, I'm sorry, but that's a no-go. We can't patch every single bridge
> > driver to support this hack. We need a solution implemented at another
> > level that will not spread throughout the whole subsystem.
> 
> it could be possible to make a better helper.. but really there aren't
> *that* many bridge drivers
> 
> suggestions ofc welcome, but I think one way or another we are going
> to need to patch bridges by the time we get to adding ACPI support, so
> really trivial couple line patches to the handful of bridges we have
> isn't really something that worries me

It's only one right now as that's the only one you care about, but
before we'll have time to blink, it will be another one, and another
one, ... Sorry, that's a no-go for me.
Rob Clark June 30, 2019, 10:04 p.m. UTC | #4 
On Sun, Jun 30, 2019 at 2:58 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> On Sun, Jun 30, 2019 at 02:50:59PM -0700, Rob Clark wrote:
> > On Sun, Jun 30, 2019 at 2:17 PM Laurent Pinchart wrote:
> > > On Sun, Jun 30, 2019 at 01:36:08PM -0700, Rob Clark wrote:
> > > > From: Rob Clark <robdclark@chromium.org>
> > > >
> > > > Use the drm_of_find_panel_id() helper to decide which endpoint to use
> > > > when looking up panel.  This way we can support devices that have
> > > > multiple possible panels, such as the aarch64 laptops.
> > > >
> > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > ---
> > > >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > index 2719d9c0864b..56c66a43f1a6 100644
> > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > @@ -790,7 +790,7 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> > > >                             const struct i2c_device_id *id)
> > > >  {
> > > >       struct ti_sn_bridge *pdata;
> > > > -     int ret;
> > > > +     int ret, panel_id;
> > > >
> > > >       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> > > >               DRM_ERROR("device doesn't support I2C\n");
> > > > @@ -811,7 +811,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> > > >
> > > >       pdata->dev = &client->dev;
> > > >
> > > > -     ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
> > > > +     panel_id = drm_of_find_panel_id();
> > > > +     ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, panel_id,
> > > >                                         &pdata->panel, NULL);
> > > >       if (ret) {
> > > >               DRM_ERROR("could not find any panel node\n");
> > >
> > > No, I'm sorry, but that's a no-go. We can't patch every single bridge
> > > driver to support this hack. We need a solution implemented at another
> > > level that will not spread throughout the whole subsystem.
> >
> > it could be possible to make a better helper.. but really there aren't
> > *that* many bridge drivers
> >
> > suggestions ofc welcome, but I think one way or another we are going
> > to need to patch bridges by the time we get to adding ACPI support, so
> > really trivial couple line patches to the handful of bridges we have
> > isn't really something that worries me
>
> It's only one right now as that's the only one you care about, but
> before we'll have time to blink, it will be another one, and another
> one, ... Sorry, that's a no-go for me.

I could ofc add helper call to all the existing bridges.. that seemed
a bit overkill for v1 patchset

BR,
-R
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 2719d9c0864b..56c66a43f1a6 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -790,7 +790,7 @@  static int ti_sn_bridge_probe(struct i2c_client *client,
 			      const struct i2c_device_id *id)
 {
 	struct ti_sn_bridge *pdata;
-	int ret;
+	int ret, panel_id;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		DRM_ERROR("device doesn't support I2C\n");
@@ -811,7 +811,8 @@  static int ti_sn_bridge_probe(struct i2c_client *client,
 
 	pdata->dev = &client->dev;
 
-	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
+	panel_id = drm_of_find_panel_id();
+	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, panel_id,
 					  &pdata->panel, NULL);
 	if (ret) {
 		DRM_ERROR("could not find any panel node\n");