Re: [PATCH] drm/arcpgu: Accommodate adv7511 switch to DRM bridge

2016-10-15 Thread Archit Taneja

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

2016-10-15 Thread Archit Taneja

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

2016-10-14 Thread Eugeniy Paltsev
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

2016-10-14 Thread Eugeniy Paltsev
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 =