Re: [PATCH] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
Hi, On 10/14/2016 6:57 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. The conversion doesn't seem entirely correct. Some comments below. 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@9e00 Path: (null) CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-1-gb5642252fa01-dirty #8 task: 9a058000 task.stack: 9a032000 [ECR ]: 0x00220100 => Invalid Read @ 0x0004 by insn @ 0x803934e8 [EFA ]: 0x0004 [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230 [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230 [STAT32]: 0x0846 : K DE E2 E1 BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x r00: 0x8063c118 r01: 0x805b98ac r02: 0x0b11 r03: 0x r04: 0x9a010f54 r05: 0x r06: 0x0001 r07: 0x r08: 0x0028 r09: 0x0001 r10: 0x0007 r11: 0x0054 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--- drivers/gpu/drm/arc/arcpgu_hdmi.c | 144 +- 1 file changed, 35 insertions(+), 109 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c index b7a8b2a..48a6c63 100644 --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c @@ -14,104 +14,54 @@ * */ +#include #include #include #include #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) +static void arcpgu_drm_i2c_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { - 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(>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; - } + struct drm_bridge *bridge = encoder->bridge; - sfuncs = slave->slave_funcs; - if (sfuncs && sfuncs->detect) - return sfuncs->detect(>base, connector); - - dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); - return status; + bridge->funcs->mode_set(bridge, mode, adjusted_mode); The bridge functions shouldn't really be called by the kms driver. They're called automatically by the drm core for the bridge attached to the encoder. See mode_fixup in drivers/gpu/drm/drm_atomic_helper.c } -static void arcpgu_drm_connector_destroy(struct drm_connector *connector) -{ - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} - -static const struct
Re: [PATCH] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
Hi, On 10/14/2016 6:57 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. The conversion doesn't seem entirely correct. Some comments below. 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@9e00 Path: (null) CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-1-gb5642252fa01-dirty #8 task: 9a058000 task.stack: 9a032000 [ECR ]: 0x00220100 => Invalid Read @ 0x0004 by insn @ 0x803934e8 [EFA ]: 0x0004 [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230 [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230 [STAT32]: 0x0846 : K DE E2 E1 BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x r00: 0x8063c118 r01: 0x805b98ac r02: 0x0b11 r03: 0x r04: 0x9a010f54 r05: 0x r06: 0x0001 r07: 0x r08: 0x0028 r09: 0x0001 r10: 0x0007 r11: 0x0054 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 --- drivers/gpu/drm/arc/arcpgu_hdmi.c | 144 +- 1 file changed, 35 insertions(+), 109 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c index b7a8b2a..48a6c63 100644 --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c @@ -14,104 +14,54 @@ * */ +#include #include #include #include #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) +static void arcpgu_drm_i2c_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { - 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(>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; - } + struct drm_bridge *bridge = encoder->bridge; - sfuncs = slave->slave_funcs; - if (sfuncs && sfuncs->detect) - return sfuncs->detect(>base, connector); - - dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); - return status; + bridge->funcs->mode_set(bridge, mode, adjusted_mode); The bridge functions shouldn't really be called by the kms driver. They're called automatically by the drm core for the bridge attached to the encoder. See mode_fixup in drivers/gpu/drm/drm_atomic_helper.c } -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
[PATCH] 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@9e00 Path: (null) CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-1-gb5642252fa01-dirty #8 task: 9a058000 task.stack: 9a032000 [ECR ]: 0x00220100 => Invalid Read @ 0x0004 by insn @ 0x803934e8 [EFA ]: 0x0004 [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230 [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230 [STAT32]: 0x0846 : K DE E2 E1 BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x r00: 0x8063c118 r01: 0x805b98ac r02: 0x0b11 r03: 0x r04: 0x9a010f54 r05: 0x r06: 0x0001 r07: 0x r08: 0x0028 r09: 0x0001 r10: 0x0007 r11: 0x0054 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--- drivers/gpu/drm/arc/arcpgu_hdmi.c | 144 +- 1 file changed, 35 insertions(+), 109 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c index b7a8b2a..48a6c63 100644 --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c @@ -14,104 +14,54 @@ * */ +#include #include #include #include #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) +static void arcpgu_drm_i2c_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { - 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(>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; - } + struct drm_bridge *bridge = encoder->bridge; - sfuncs = slave->slave_funcs; - if (sfuncs && sfuncs->detect) - return sfuncs->detect(>base, connector); - - dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); - return status; + bridge->funcs->mode_set(bridge, mode, adjusted_mode); } -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 =
[PATCH] 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@9e00 Path: (null) CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-1-gb5642252fa01-dirty #8 task: 9a058000 task.stack: 9a032000 [ECR ]: 0x00220100 => Invalid Read @ 0x0004 by insn @ 0x803934e8 [EFA ]: 0x0004 [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230 [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230 [STAT32]: 0x0846 : K DE E2 E1 BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x r00: 0x8063c118 r01: 0x805b98ac r02: 0x0b11 r03: 0x r04: 0x9a010f54 r05: 0x r06: 0x0001 r07: 0x r08: 0x0028 r09: 0x0001 r10: 0x0007 r11: 0x0054 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 --- drivers/gpu/drm/arc/arcpgu_hdmi.c | 144 +- 1 file changed, 35 insertions(+), 109 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c index b7a8b2a..48a6c63 100644 --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c @@ -14,104 +14,54 @@ * */ +#include #include #include #include #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) +static void arcpgu_drm_i2c_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { - 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(>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; - } + struct drm_bridge *bridge = encoder->bridge; - sfuncs = slave->slave_funcs; - if (sfuncs && sfuncs->detect) - return sfuncs->detect(>base, connector); - - dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); - return status; + bridge->funcs->mode_set(bridge, mode, adjusted_mode); } -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 =