2016-10-19 15:01:40

by Eugeniy Paltsev

[permalink] [raw]
Subject: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

ARC PGU driver starts crashing on initialization after
'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
of "drm_i2c_encoder_driver" structure, which doesn't exist after
adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
So, when we call "encoder_init" function from this structure driver
crashes.

Bootlog:
------------------------------------->8--------------------------------
[drm] Initialized drm 1.1.0 20060810
arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab
arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e000000
Path: (null)
CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-00001-gb5642252fa01-dirty #8
task: 9a058000 task.stack: 9a032000

[ECR ]: 0x00220100 => Invalid Read @ 0x00000004 by insn @ 0x803934e8
[EFA ]: 0x00000004
[BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230
[ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230
[STAT32]: 0x00000846 : K DE E2 E1
BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x00000000
LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x00000000
r00: 0x8063c118 r01: 0x805b98ac r02: 0x00000b11
r03: 0x00000000 r04: 0x9a010f54 r05: 0x00000000
r06: 0x00000001 r07: 0x00000000 r08: 0x00000028
r09: 0x00000001 r10: 0x00000007 r11: 0x00000054
r12: 0x720a3033

Stack Trace:
drm_atomic_helper_connector_dpms+0xa4/0x230
arcpgu_drm_hdmi_init+0xbc/0x228
arcpgu_probe+0x168/0x244
platform_drv_probe+0x26/0x64
really_probe+0x1f0/0x32c
__driver_attach+0xa8/0xd0
bus_for_each_dev+0x3c/0x74
bus_add_driver+0xc2/0x184
driver_register+0x50/0xec
do_one_initcall+0x3a/0x120
kernel_init_freeable+0x108/0x1a0
------------------------------------->8--------------------------------

Fix ARC PGU driver to be able work with drm bridge hdmi encoder
interface. The hdmi connector code isn't needed anymore as we expect
the adv7511 bridge driver to create/manage the connector.

Signed-off-by: Eugeniy Paltsev <[email protected]>
---
Changes for v2:
- remove bridge functions call from kms driver
- get rid of drm_encoder_slave interface
- coding style cleanup

drivers/gpu/drm/arc/arcpgu_hdmi.c | 159 ++++----------------------------------
1 file changed, 17 insertions(+), 142 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index b7a8b2a..b69c66b 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -14,170 +14,45 @@
*
*/

-#include <drm/drm_crtc_helper.h>
+#include <drm/drm_crtc.h>
#include <drm/drm_encoder_slave.h>
-#include <drm/drm_atomic_helper.h>

#include "arcpgu.h"

-struct arcpgu_drm_connector {
- struct drm_connector connector;
- struct drm_encoder_slave *encoder_slave;
-};
-
-static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
-{
- const struct drm_encoder_slave_funcs *sfuncs;
- struct drm_encoder_slave *slave;
- struct arcpgu_drm_connector *con =
- container_of(connector, struct arcpgu_drm_connector, connector);
-
- slave = con->encoder_slave;
- if (slave == NULL) {
- dev_err(connector->dev->dev,
- "connector_get_modes: cannot find slave encoder for connector\n");
- return 0;
- }
-
- sfuncs = slave->slave_funcs;
- if (sfuncs->get_modes == NULL)
- return 0;
-
- return sfuncs->get_modes(&slave->base, connector);
-}
-
-static enum drm_connector_status
-arcpgu_drm_connector_detect(struct drm_connector *connector, bool force)
-{
- enum drm_connector_status status = connector_status_unknown;
- const struct drm_encoder_slave_funcs *sfuncs;
- struct drm_encoder_slave *slave;
-
- struct arcpgu_drm_connector *con =
- container_of(connector, struct arcpgu_drm_connector, connector);
-
- slave = con->encoder_slave;
- if (slave == NULL) {
- dev_err(connector->dev->dev,
- "connector_detect: cannot find slave encoder for connector\n");
- return status;
- }
-
- sfuncs = slave->slave_funcs;
- if (sfuncs && sfuncs->detect)
- return sfuncs->detect(&slave->base, connector);
-
- dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n");
- return status;
-}
-
-static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
-{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-
-static const struct drm_connector_helper_funcs
-arcpgu_drm_connector_helper_funcs = {
- .get_modes = arcpgu_drm_connector_get_modes,
-};
-
-static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
- .dpms = drm_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .detect = arcpgu_drm_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = arcpgu_drm_connector_destroy,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static struct drm_encoder_helper_funcs arcpgu_drm_encoder_helper_funcs = {
- .dpms = drm_i2c_encoder_dpms,
- .mode_fixup = drm_i2c_encoder_mode_fixup,
- .mode_set = drm_i2c_encoder_mode_set,
- .prepare = drm_i2c_encoder_prepare,
- .commit = drm_i2c_encoder_commit,
- .detect = drm_i2c_encoder_detect,
-};
-
static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
.destroy = drm_encoder_cleanup,
};

int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
{
- struct arcpgu_drm_connector *arcpgu_connector;
- struct drm_i2c_encoder_driver *driver;
- struct drm_encoder_slave *encoder;
- struct drm_connector *connector;
- struct i2c_client *i2c_slave;
- int ret;
+ struct drm_encoder *encoder;
+ struct drm_bridge *bridge;
+
+ int ret = 0;

encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL);
if (encoder == NULL)
return -ENOMEM;

- i2c_slave = of_find_i2c_device_by_node(np);
- if (!i2c_slave || !i2c_get_clientdata(i2c_slave)) {
- dev_err(drm->dev, "failed to find i2c slave encoder\n");
- return -EPROBE_DEFER;
- }
-
- if (i2c_slave->dev.driver == NULL) {
- dev_err(drm->dev, "failed to find i2c slave driver\n");
+ /* Locate drm bridge from the hdmi encoder DT node */
+ bridge = of_drm_find_bridge(np);
+ if (!bridge)
return -EPROBE_DEFER;
- }

- driver =
- to_drm_i2c_encoder_driver(to_i2c_driver(i2c_slave->dev.driver));
- ret = driver->encoder_init(i2c_slave, drm, encoder);
- if (ret) {
- dev_err(drm->dev, "failed to initialize i2c encoder slave\n");
- return ret;
- }
-
- encoder->base.possible_crtcs = 1;
- encoder->base.possible_clones = 0;
- ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs,
+ encoder->possible_crtcs = 1;
+ encoder->possible_clones = 0;
+ ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);
if (ret)
return ret;

- drm_encoder_helper_add(&encoder->base,
- &arcpgu_drm_encoder_helper_funcs);
-
- arcpgu_connector = devm_kzalloc(drm->dev, sizeof(*arcpgu_connector),
- GFP_KERNEL);
- if (!arcpgu_connector) {
- ret = -ENOMEM;
- goto error_encoder_cleanup;
- }
-
- connector = &arcpgu_connector->connector;
- drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs);
- ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs,
- DRM_MODE_CONNECTOR_HDMIA);
- if (ret < 0) {
- dev_err(drm->dev, "failed to initialize drm connector\n");
- goto error_encoder_cleanup;
- }
+ /* Link drm_bridge to encoder */
+ bridge->encoder = encoder;
+ encoder->bridge = bridge;

- ret = drm_mode_connector_attach_encoder(connector, &encoder->base);
- if (ret < 0) {
- dev_err(drm->dev, "could not attach connector to encoder\n");
- drm_connector_unregister(connector);
- goto error_connector_cleanup;
- }
-
- arcpgu_connector->encoder_slave = encoder;
-
- return 0;
-
-error_connector_cleanup:
- drm_connector_cleanup(connector);
+ ret = drm_bridge_attach(drm, bridge);
+ if (ret)
+ drm_encoder_cleanup(encoder);

-error_encoder_cleanup:
- drm_encoder_cleanup(&encoder->base);
return ret;
}
--
2.5.5


2016-10-19 14:21:33

by Alexey Brodkin

[permalink] [raw]
Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

Hi Archit, all,

On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote:
>
> On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
> >
> > ARC PGU driver starts crashing on initialization after
> > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
> > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
> > of "drm_i2c_encoder_driver" structure, which doesn't exist after
> > adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
> > So, when we call "encoder_init" function from this structure driver
> > crashes.

[snip]

> Looks good now.
>
> Reviewed-by: Archit Taneja <[email protected]>

And IMHO it would be really good to get this one back-ported to 4.8
because it really fixes kernel crash if ARC PGU driver is used.

It might be a bit of a problem because patch itself a little-bit larger
than formal requirement for stable backports but let's see if it gets accepted.

-Alexey

2016-10-19 15:11:43

by Archit Taneja

[permalink] [raw]
Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge



On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
> ARC PGU driver starts crashing on initialization after
> 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
> This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
> of "drm_i2c_encoder_driver" structure, which doesn't exist after
> adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
> So, when we call "encoder_init" function from this structure driver
> crashes.
>
> Bootlog:
> ------------------------------------->8--------------------------------
> [drm] Initialized drm 1.1.0 20060810
> arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab
> arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e000000
> Path: (null)
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-00001-gb5642252fa01-dirty #8
> task: 9a058000 task.stack: 9a032000
>
> [ECR ]: 0x00220100 => Invalid Read @ 0x00000004 by insn @ 0x803934e8
> [EFA ]: 0x00000004
> [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230
> [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230
> [STAT32]: 0x00000846 : K DE E2 E1
> BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x00000000
> LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x00000000
> r00: 0x8063c118 r01: 0x805b98ac r02: 0x00000b11
> r03: 0x00000000 r04: 0x9a010f54 r05: 0x00000000
> r06: 0x00000001 r07: 0x00000000 r08: 0x00000028
> r09: 0x00000001 r10: 0x00000007 r11: 0x00000054
> r12: 0x720a3033
>
> Stack Trace:
> drm_atomic_helper_connector_dpms+0xa4/0x230
> arcpgu_drm_hdmi_init+0xbc/0x228
> arcpgu_probe+0x168/0x244
> platform_drv_probe+0x26/0x64
> really_probe+0x1f0/0x32c
> __driver_attach+0xa8/0xd0
> bus_for_each_dev+0x3c/0x74
> bus_add_driver+0xc2/0x184
> driver_register+0x50/0xec
> do_one_initcall+0x3a/0x120
> kernel_init_freeable+0x108/0x1a0
> ------------------------------------->8--------------------------------
>
> Fix ARC PGU driver to be able work with drm bridge hdmi encoder
> interface. The hdmi connector code isn't needed anymore as we expect
> the adv7511 bridge driver to create/manage the connector.

Looks good now.

Reviewed-by: Archit Taneja <[email protected]>

>
> Signed-off-by: Eugeniy Paltsev <[email protected]>
> ---
> Changes for v2:
> - remove bridge functions call from kms driver
> - get rid of drm_encoder_slave interface
> - coding style cleanup
>
> drivers/gpu/drm/arc/arcpgu_hdmi.c | 159 ++++----------------------------------
> 1 file changed, 17 insertions(+), 142 deletions(-)
>
> diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> index b7a8b2a..b69c66b 100644
> --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> @@ -14,170 +14,45 @@
> *
> */
>
> -#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_crtc.h>
> #include <drm/drm_encoder_slave.h>
> -#include <drm/drm_atomic_helper.h>
>
> #include "arcpgu.h"
>
> -struct arcpgu_drm_connector {
> - struct drm_connector connector;
> - struct drm_encoder_slave *encoder_slave;
> -};
> -
> -static int arcpgu_drm_connector_get_modes(struct drm_connector *connector)
> -{
> - const struct drm_encoder_slave_funcs *sfuncs;
> - struct drm_encoder_slave *slave;
> - struct arcpgu_drm_connector *con =
> - container_of(connector, struct arcpgu_drm_connector, connector);
> -
> - slave = con->encoder_slave;
> - if (slave == NULL) {
> - dev_err(connector->dev->dev,
> - "connector_get_modes: cannot find slave encoder for connector\n");
> - return 0;
> - }
> -
> - sfuncs = slave->slave_funcs;
> - if (sfuncs->get_modes == NULL)
> - return 0;
> -
> - return sfuncs->get_modes(&slave->base, connector);
> -}
> -
> -static enum drm_connector_status
> -arcpgu_drm_connector_detect(struct drm_connector *connector, bool force)
> -{
> - enum drm_connector_status status = connector_status_unknown;
> - const struct drm_encoder_slave_funcs *sfuncs;
> - struct drm_encoder_slave *slave;
> -
> - struct arcpgu_drm_connector *con =
> - container_of(connector, struct arcpgu_drm_connector, connector);
> -
> - slave = con->encoder_slave;
> - if (slave == NULL) {
> - dev_err(connector->dev->dev,
> - "connector_detect: cannot find slave encoder for connector\n");
> - return status;
> - }
> -
> - sfuncs = slave->slave_funcs;
> - if (sfuncs && sfuncs->detect)
> - return sfuncs->detect(&slave->base, connector);
> -
> - dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n");
> - return status;
> -}
> -
> -static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
> -{
> - drm_connector_unregister(connector);
> - drm_connector_cleanup(connector);
> -}
> -
> -static const struct drm_connector_helper_funcs
> -arcpgu_drm_connector_helper_funcs = {
> - .get_modes = arcpgu_drm_connector_get_modes,
> -};
> -
> -static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
> - .dpms = drm_helper_connector_dpms,
> - .reset = drm_atomic_helper_connector_reset,
> - .detect = arcpgu_drm_connector_detect,
> - .fill_modes = drm_helper_probe_single_connector_modes,
> - .destroy = arcpgu_drm_connector_destroy,
> - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
> -static struct drm_encoder_helper_funcs arcpgu_drm_encoder_helper_funcs = {
> - .dpms = drm_i2c_encoder_dpms,
> - .mode_fixup = drm_i2c_encoder_mode_fixup,
> - .mode_set = drm_i2c_encoder_mode_set,
> - .prepare = drm_i2c_encoder_prepare,
> - .commit = drm_i2c_encoder_commit,
> - .detect = drm_i2c_encoder_detect,
> -};
> -
> static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
> .destroy = drm_encoder_cleanup,
> };
>
> int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np)
> {
> - struct arcpgu_drm_connector *arcpgu_connector;
> - struct drm_i2c_encoder_driver *driver;
> - struct drm_encoder_slave *encoder;
> - struct drm_connector *connector;
> - struct i2c_client *i2c_slave;
> - int ret;
> + struct drm_encoder *encoder;
> + struct drm_bridge *bridge;
> +
> + int ret = 0;
>
> encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL);
> if (encoder == NULL)
> return -ENOMEM;
>
> - i2c_slave = of_find_i2c_device_by_node(np);
> - if (!i2c_slave || !i2c_get_clientdata(i2c_slave)) {
> - dev_err(drm->dev, "failed to find i2c slave encoder\n");
> - return -EPROBE_DEFER;
> - }
> -
> - if (i2c_slave->dev.driver == NULL) {
> - dev_err(drm->dev, "failed to find i2c slave driver\n");
> + /* Locate drm bridge from the hdmi encoder DT node */
> + bridge = of_drm_find_bridge(np);
> + if (!bridge)
> return -EPROBE_DEFER;
> - }
>
> - driver =
> - to_drm_i2c_encoder_driver(to_i2c_driver(i2c_slave->dev.driver));
> - ret = driver->encoder_init(i2c_slave, drm, encoder);
> - if (ret) {
> - dev_err(drm->dev, "failed to initialize i2c encoder slave\n");
> - return ret;
> - }
> -
> - encoder->base.possible_crtcs = 1;
> - encoder->base.possible_clones = 0;
> - ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs,
> + encoder->possible_crtcs = 1;
> + encoder->possible_clones = 0;
> + ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
> DRM_MODE_ENCODER_TMDS, NULL);
> if (ret)
> return ret;
>
> - drm_encoder_helper_add(&encoder->base,
> - &arcpgu_drm_encoder_helper_funcs);
> -
> - arcpgu_connector = devm_kzalloc(drm->dev, sizeof(*arcpgu_connector),
> - GFP_KERNEL);
> - if (!arcpgu_connector) {
> - ret = -ENOMEM;
> - goto error_encoder_cleanup;
> - }
> -
> - connector = &arcpgu_connector->connector;
> - drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs);
> - ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs,
> - DRM_MODE_CONNECTOR_HDMIA);
> - if (ret < 0) {
> - dev_err(drm->dev, "failed to initialize drm connector\n");
> - goto error_encoder_cleanup;
> - }
> + /* Link drm_bridge to encoder */
> + bridge->encoder = encoder;
> + encoder->bridge = bridge;
>
> - ret = drm_mode_connector_attach_encoder(connector, &encoder->base);
> - if (ret < 0) {
> - dev_err(drm->dev, "could not attach connector to encoder\n");
> - drm_connector_unregister(connector);
> - goto error_connector_cleanup;
> - }
> -
> - arcpgu_connector->encoder_slave = encoder;
> -
> - return 0;
> -
> -error_connector_cleanup:
> - drm_connector_cleanup(connector);
> + ret = drm_bridge_attach(drm, bridge);
> + if (ret)
> + drm_encoder_cleanup(encoder);
>
> -error_encoder_cleanup:
> - drm_encoder_cleanup(&encoder->base);
> return ret;
> }
>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2016-10-21 14:56:33

by Ramiro Oliveira

[permalink] [raw]
Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
> ARC PGU driver starts crashing on initialization after
> 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
> This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
> of "drm_i2c_encoder_driver" structure, which doesn't exist after
> adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
> So, when we call "encoder_init" function from this structure driver
> crashes.
>
> Bootlog:
> ------------------------------------->8--------------------------------
> [drm] Initialized drm 1.1.0 20060810
> arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab
> arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e000000
> Path: (null)
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-00001-gb5642252fa01-dirty #8
> task: 9a058000 task.stack: 9a032000
>
> [ECR ]: 0x00220100 => Invalid Read @ 0x00000004 by insn @ 0x803934e8
> [EFA ]: 0x00000004
> [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230
> [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230
> [STAT32]: 0x00000846 : K DE E2 E1
> BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x00000000
> LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x00000000
> r00: 0x8063c118 r01: 0x805b98ac r02: 0x00000b11
> r03: 0x00000000 r04: 0x9a010f54 r05: 0x00000000
> r06: 0x00000001 r07: 0x00000000 r08: 0x00000028
> r09: 0x00000001 r10: 0x00000007 r11: 0x00000054
> r12: 0x720a3033
>
> Stack Trace:
> drm_atomic_helper_connector_dpms+0xa4/0x230
> arcpgu_drm_hdmi_init+0xbc/0x228
> arcpgu_probe+0x168/0x244
> platform_drv_probe+0x26/0x64
> really_probe+0x1f0/0x32c
> __driver_attach+0xa8/0xd0
> bus_for_each_dev+0x3c/0x74
> bus_add_driver+0xc2/0x184
> driver_register+0x50/0xec
> do_one_initcall+0x3a/0x120
> kernel_init_freeable+0x108/0x1a0
> ------------------------------------->8--------------------------------
>
> Fix ARC PGU driver to be able work with drm bridge hdmi encoder
> interface. The hdmi connector code isn't needed anymore as we expect
> the adv7511 bridge driver to create/manage the connector.

Tested-by: Ramiro Oliveira <[email protected]>

2016-10-24 18:41:26

by Alexey Brodkin

[permalink] [raw]
Subject: RE: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

Hi Daniel,

> -----Original Message-----
> From: linux-snps-arc [mailto:[email protected]] On Behalf Of Alexey Brodkin
> Sent: 19 ??????? 2016 ?. 12:33
> To: [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
>
> Hi Archit, all,
>
> On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote:
> >
> > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
> > >
> > > ARC PGU driver starts crashing on initialization after
> > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
> > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
> > > of "drm_i2c_encoder_driver" structure, which doesn't exist after
> > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
> > > So, when we call "encoder_init" function from this structure driver
> > > crashes.
>
> [snip]
>
> > Looks good now.
> >
> > Reviewed-by: Archit Taneja <[email protected]>
>
> And IMHO it would be really good to get this one back-ported to 4.8
> because it really fixes kernel crash if ARC PGU driver is used.
>
> It might be a bit of a problem because patch itself a little-bit larger
> than formal requirement for stable backports but let's see if it gets accepted.

Could you please pick this one up?
I may alternatively send a pull-request to David but not sure if 1 patch worth it.

Also if that's not really too late it would be good to get this one in 4.9 since the patch
In question fixes a real driver crash on its instantiation.
Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount
of changes I think there's barely a chance for it it to be accepted in stable branches...
which in its turn makes at least 4.9 very desirable.

-Alexey

2016-11-02 12:23:51

by Alexey Brodkin

[permalink] [raw]
Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

Hi Daniel, David,

On Mon, 2016-10-24 at 18:33 +0000, Alexey Brodkin wrote:
> Hi Daniel,
>
> >
> > -----Original Message-----
> > From: linux-snps-arc [mailto:[email protected]] On Behalf Of Alexey Brodkin
> > Sent: 19 октября 2016 г. 12:33
> > To: [email protected]; [email protected]; [email protected]
> > Cc: [email protected]; [email protected]
> > Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
> >
> > Hi Archit, all,
> >
> > On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote:
> > >
> > >
> > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
> > > >
> > > >
> > > > ARC PGU driver starts crashing on initialization after
> > > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
> > > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
> > > > of "drm_i2c_encoder_driver" structure, which doesn't exist after
> > > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
> > > > So, when we call "encoder_init" function from this structure driver
> > > > crashes.
> >
> > [snip]
> >
> > >
> > > Looks good now.
> > >
> > > Reviewed-by: Archit Taneja <[email protected]>
> >
> > And IMHO it would be really good to get this one back-ported to 4.8
> > because it really fixes kernel crash if ARC PGU driver is used.
> >
> > It might be a bit of a problem because patch itself a little-bit larger
> > than formal requirement for stable backports but let's see if it gets accepted.
>
> Could you please pick this one up?
> I may alternatively send a pull-request to David but not sure if 1 patch worth it.
>
> Also if that's not really too late it would be good to get this one in 4.9 since the patch
> In question fixes a real driver crash on its instantiation.
> Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount
> of changes I think there's barely a chance for it it to be accepted in stable branches...
> which in its turn makes at least 4.9 very desirable.

Any chance this one gets accepted anytime soon?

Regards,
Alexey

2016-11-10 11:06:47

by Alexey Brodkin

[permalink] [raw]
Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

Hi Daniel, David,

On Wed, 2016-11-02 at 12:23 +0000, Alexey Brodkin wrote:
> Hi Daniel, David,
>
> On Mon, 2016-10-24 at 18:33 +0000, Alexey Brodkin wrote:
> >
> > Hi Daniel,
> >
> > >
> > >
> > > -----Original Message-----
> > > From: linux-snps-arc [mailto:[email protected]] On Behalf Of Alexey Brodkin
> > > Sent: 19 октября 2016 г. 12:33
> > > To: [email protected]; [email protected]; [email protected]
> > > Cc: [email protected]; [email protected]
> > > Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
> > >
> > > Hi Archit, all,
> > >
> > > On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote:
> > > >
> > > >
> > > >
> > > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
> > > > >
> > > > >
> > > > >
> > > > > ARC PGU driver starts crashing on initialization after
> > > > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
> > > > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
> > > > > of "drm_i2c_encoder_driver" structure, which doesn't exist after
> > > > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
> > > > > So, when we call "encoder_init" function from this structure driver
> > > > > crashes.
> > >
> > > [snip]
> > >
> > > >
> > > >
> > > > Looks good now.
> > > >
> > > > Reviewed-by: Archit Taneja <[email protected]>
> > >
> > > And IMHO it would be really good to get this one back-ported to 4.8
> > > because it really fixes kernel crash if ARC PGU driver is used.
> > >
> > > It might be a bit of a problem because patch itself a little-bit larger
> > > than formal requirement for stable backports but let's see if it gets accepted.
> >
> > Could you please pick this one up?
> > I may alternatively send a pull-request to David but not sure if 1 patch worth it.
> >
> > Also if that's not really too late it would be good to get this one in 4.9 since the patch
> > In question fixes a real driver crash on its instantiation.
> > Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount
> > of changes I think there's barely a chance for it it to be accepted in stable branches...
> > which in its turn makes at least 4.9 very desirable.
>
> Any chance this one gets accepted anytime soon?

Please treat this as another polite reminder to apply this patch.
If you prefer I may send a pull-request otherwise.

Regards,
Alexey

2016-11-10 23:53:52

by Dave Airlie

[permalink] [raw]
Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

On 10 November 2016 at 21:06, Alexey Brodkin
<[email protected]> wrote:
> Hi Daniel, David,
>
> On Wed, 2016-11-02 at 12:23 +0000, Alexey Brodkin wrote:
>> Hi Daniel, David,
>>
>> On Mon, 2016-10-24 at 18:33 +0000, Alexey Brodkin wrote:
>> >
>> > Hi Daniel,
>> >
>> > >
>> > >
>> > > -----Original Message-----
>> > > From: linux-snps-arc [mailto:[email protected]] On Behalf Of Alexey Brodkin
>> > > Sent: 19 октября 2016 г. 12:33
>> > > To: [email protected]; [email protected]; [email protected]
>> > > Cc: [email protected]; [email protected]
>> > > Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
>> > >
>> > > Hi Archit, all,
>> > >
>> > > On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote:
>> > > >
>> > > >
>> > > >
>> > > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote:
>> > > > >
>> > > > >
>> > > > >
>> > > > > ARC PGU driver starts crashing on initialization after
>> > > > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")'
>> > > > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer
>> > > > > of "drm_i2c_encoder_driver" structure, which doesn't exist after
>> > > > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge.
>> > > > > So, when we call "encoder_init" function from this structure driver
>> > > > > crashes.
>> > >
>> > > [snip]
>> > >
>> > > >
>> > > >
>> > > > Looks good now.
>> > > >
>> > > > Reviewed-by: Archit Taneja <[email protected]>
>> > >
>> > > And IMHO it would be really good to get this one back-ported to 4.8
>> > > because it really fixes kernel crash if ARC PGU driver is used.
>> > >
>> > > It might be a bit of a problem because patch itself a little-bit larger
>> > > than formal requirement for stable backports but let's see if it gets accepted.
>> >
>> > Could you please pick this one up?
>> > I may alternatively send a pull-request to David but not sure if 1 patch worth it.
>> >
>> > Also if that's not really too late it would be good to get this one in 4.9 since the patch
>> > In question fixes a real driver crash on its instantiation.
>> > Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount
>> > of changes I think there's barely a chance for it it to be accepted in stable branches...
>> > which in its turn makes at least 4.9 very desirable.
>>
>> Any chance this one gets accepted anytime soon?
>
> Please treat this as another polite reminder to apply this patch.
> If you prefer I may send a pull-request otherwise.

Please send a pull request for -fixes.

For everyone else, pull requests for single patches is not overkill,
it fits into my workflow a lot better.

Dave.