| Message ID | 1495110921-4032-1-git-send-email-boris.brezillon@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18/05/2017 at 14:35:21 +0200, Boris Brezillon wrote: > drm_of_find_panel_or_bridge() is expecting np to point to the encoder > node, not the bridge or panel this encoder is feeding. > Moreover, the endpoint parameter passed to drm_of_find_panel_or_bridge() > is always set to zero, which prevents us from probing all outputs. > > We also move the atmel_hlcdc_rgb_output allocation after the > panel/bridge detection to avoid useless allocations. > > Reported-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> Tested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Fixes: ebc944613567 ("drm: convert drivers to use drm_of_find_panel_or_bridge") > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 36 +++++++++--------------- > 1 file changed, 14 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > index 65a3bd7a0c00..423dda2785d4 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > @@ -152,8 +152,7 @@ static const struct drm_connector_funcs atmel_hlcdc_panel_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > -static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > - const struct device_node *np) > +static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint) > { > struct atmel_hlcdc_dc *dc = dev->dev_private; > struct atmel_hlcdc_rgb_output *output; > @@ -161,6 +160,11 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > struct drm_bridge *bridge; > int ret; > > + ret = drm_of_find_panel_or_bridge(dev->dev->of_node, 0, endpoint, > + &panel, &bridge); > + if (ret) > + return ret; > + > output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL); > if (!output) > return -EINVAL; > @@ -177,10 +181,6 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > > output->encoder.possible_crtcs = 0x1; > > - ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); > - if (ret) > - return ret; > - > if (panel) { > output->connector.dpms = DRM_MODE_DPMS_OFF; > output->connector.polled = DRM_CONNECTOR_POLL_CONNECT; > @@ -220,22 +220,14 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > > int atmel_hlcdc_create_outputs(struct drm_device *dev) > { > - struct device_node *remote; > - int ret = -ENODEV; > - int endpoint = 0; > - > - while (true) { > - /* Loop thru possible multiple connections to the output */ > - remote = of_graph_get_remote_node(dev->dev->of_node, 0, > - endpoint++); > - if (!remote) > - break; > - > - ret = atmel_hlcdc_attach_endpoint(dev, remote); > - of_node_put(remote); > - if (ret) > - return ret; > - } > + int endpoint, ret = 0; > + > + for (endpoint = 0; !ret; endpoint++) > + ret = atmel_hlcdc_attach_endpoint(dev, endpoint); > + > + /* At least one device was successfully attached.*/ > + if (ret == -ENODEV && endpoint) > + return 0; > > return ret; > } > -- > 2.7.4 >
On Thu, May 18, 2017 at 02:43:07PM +0200, Alexandre Belloni wrote: > On 18/05/2017 at 14:35:21 +0200, Boris Brezillon wrote: > > drm_of_find_panel_or_bridge() is expecting np to point to the encoder > > node, not the bridge or panel this encoder is feeding. > > Moreover, the endpoint parameter passed to drm_of_find_panel_or_bridge() > > is always set to zero, which prevents us from probing all outputs. > > > > We also move the atmel_hlcdc_rgb_output allocation after the > > panel/bridge detection to avoid useless allocations. > > > > Reported-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Tested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> Applied to -misc-fixes, thank you Sean > > > Fixes: ebc944613567 ("drm: convert drivers to use drm_of_find_panel_or_bridge") > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > > --- > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 36 +++++++++--------------- > > 1 file changed, 14 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > > index 65a3bd7a0c00..423dda2785d4 100644 > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c > > @@ -152,8 +152,7 @@ static const struct drm_connector_funcs atmel_hlcdc_panel_connector_funcs = { > > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > }; > > > > -static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > > - const struct device_node *np) > > +static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint) > > { > > struct atmel_hlcdc_dc *dc = dev->dev_private; > > struct atmel_hlcdc_rgb_output *output; > > @@ -161,6 +160,11 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > > struct drm_bridge *bridge; > > int ret; > > > > + ret = drm_of_find_panel_or_bridge(dev->dev->of_node, 0, endpoint, > > + &panel, &bridge); > > + if (ret) > > + return ret; > > + > > output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL); > > if (!output) > > return -EINVAL; > > @@ -177,10 +181,6 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > > > > output->encoder.possible_crtcs = 0x1; > > > > - ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); > > - if (ret) > > - return ret; > > - > > if (panel) { > > output->connector.dpms = DRM_MODE_DPMS_OFF; > > output->connector.polled = DRM_CONNECTOR_POLL_CONNECT; > > @@ -220,22 +220,14 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, > > > > int atmel_hlcdc_create_outputs(struct drm_device *dev) > > { > > - struct device_node *remote; > > - int ret = -ENODEV; > > - int endpoint = 0; > > - > > - while (true) { > > - /* Loop thru possible multiple connections to the output */ > > - remote = of_graph_get_remote_node(dev->dev->of_node, 0, > > - endpoint++); > > - if (!remote) > > - break; > > - > > - ret = atmel_hlcdc_attach_endpoint(dev, remote); > > - of_node_put(remote); > > - if (ret) > > - return ret; > > - } > > + int endpoint, ret = 0; > > + > > + for (endpoint = 0; !ret; endpoint++) > > + ret = atmel_hlcdc_attach_endpoint(dev, endpoint); > > + > > + /* At least one device was successfully attached.*/ > > + if (ret == -ENODEV && endpoint) > > + return 0; > > > > return ret; > > } > > -- > > 2.7.4 > > > > -- > Alexandre Belloni, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c index 65a3bd7a0c00..423dda2785d4 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c @@ -152,8 +152,7 @@ static const struct drm_connector_funcs atmel_hlcdc_panel_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; -static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, - const struct device_node *np) +static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint) { struct atmel_hlcdc_dc *dc = dev->dev_private; struct atmel_hlcdc_rgb_output *output; @@ -161,6 +160,11 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, struct drm_bridge *bridge; int ret; + ret = drm_of_find_panel_or_bridge(dev->dev->of_node, 0, endpoint, + &panel, &bridge); + if (ret) + return ret; + output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL); if (!output) return -EINVAL; @@ -177,10 +181,6 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, output->encoder.possible_crtcs = 0x1; - ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); - if (ret) - return ret; - if (panel) { output->connector.dpms = DRM_MODE_DPMS_OFF; output->connector.polled = DRM_CONNECTOR_POLL_CONNECT; @@ -220,22 +220,14 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int atmel_hlcdc_create_outputs(struct drm_device *dev) { - struct device_node *remote; - int ret = -ENODEV; - int endpoint = 0; - - while (true) { - /* Loop thru possible multiple connections to the output */ - remote = of_graph_get_remote_node(dev->dev->of_node, 0, - endpoint++); - if (!remote) - break; - - ret = atmel_hlcdc_attach_endpoint(dev, remote); - of_node_put(remote); - if (ret) - return ret; - } + int endpoint, ret = 0; + + for (endpoint = 0; !ret; endpoint++) + ret = atmel_hlcdc_attach_endpoint(dev, endpoint); + + /* At least one device was successfully attached.*/ + if (ret == -ENODEV && endpoint) + return 0; return ret; }
drm_of_find_panel_or_bridge() is expecting np to point to the encoder node, not the bridge or panel this encoder is feeding. Moreover, the endpoint parameter passed to drm_of_find_panel_or_bridge() is always set to zero, which prevents us from probing all outputs. We also move the atmel_hlcdc_rgb_output allocation after the panel/bridge detection to avoid useless allocations. Reported-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> Fixes: ebc944613567 ("drm: convert drivers to use drm_of_find_panel_or_bridge") Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 36 +++++++++--------------- 1 file changed, 14 insertions(+), 22 deletions(-)