diff mbox

drm/arcpgu: remove drm_encoder_slave

Message ID 20180117141755.16933-1-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Jan. 17, 2018, 2:17 p.m. UTC
drm_encoder_slave is the old way to write bridge drivers, for i2c
bridges only. It's deprecated, and definitely should not be used in
new drivers. This has absolutely nothing to do with the new bridge
driver infrastructure implemented by drm_bridge.

What's even strange is that arcpgu doesn't even use any of this, it
really only wants a plain normal drm_encoder. Nuke all the surplus
real estate.

v2: Actually git add after compile testing ...

v3: Clarify commit message and stop including drm_encoder_slave.h.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/arc/arcpgu_hdmi.c |  3 ++-
 drivers/gpu/drm/arc/arcpgu_sim.c  | 16 ++++++----------
 2 files changed, 8 insertions(+), 11 deletions(-)

Comments

Daniel Vetter Jan. 30, 2018, 9:15 a.m. UTC | #1
On Wed, Jan 17, 2018 at 03:17:55PM +0100, Daniel Vetter wrote:
> drm_encoder_slave is the old way to write bridge drivers, for i2c
> bridges only. It's deprecated, and definitely should not be used in
> new drivers. This has absolutely nothing to do with the new bridge
> driver infrastructure implemented by drm_bridge.
> 
> What's even strange is that arcpgu doesn't even use any of this, it
> really only wants a plain normal drm_encoder. Nuke all the surplus
> real estate.
> 
> v2: Actually git add after compile testing ...
> 
> v3: Clarify commit message and stop including drm_encoder_slave.h.
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

I assumed this is ok and applied it to drm-misc-next.
-Daniel

> ---
>  drivers/gpu/drm/arc/arcpgu_hdmi.c |  3 ++-
>  drivers/gpu/drm/arc/arcpgu_sim.c  | 16 ++++++----------
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> index 0ce7f398bcff..977dfa55162f 100644
> --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
> +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
> @@ -15,7 +15,8 @@
>   */
>  
>  #include <drm/drm_crtc.h>
> -#include <drm/drm_encoder_slave.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_device.h>
>  
>  #include "arcpgu.h"
>  
> diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
> index bca3a678c955..b8f6f9a5dfbe 100644
> --- a/drivers/gpu/drm/arc/arcpgu_sim.c
> +++ b/drivers/gpu/drm/arc/arcpgu_sim.c
> @@ -15,7 +15,6 @@
>   */
>  
>  #include <drm/drm_crtc_helper.h>
> -#include <drm/drm_encoder_slave.h>
>  #include <drm/drm_atomic_helper.h>
>  
>  #include "arcpgu.h"
> @@ -29,7 +28,6 @@
>  
>  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)
> @@ -68,7 +66,7 @@ static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
>  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>  {
>  	struct arcpgu_drm_connector *arcpgu_connector;
> -	struct drm_encoder_slave *encoder;
> +	struct drm_encoder *encoder;
>  	struct drm_connector *connector;
>  	int ret;
>  
> @@ -76,10 +74,10 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>  	if (encoder == NULL)
>  		return -ENOMEM;
>  
> -	encoder->base.possible_crtcs = 1;
> -	encoder->base.possible_clones = 0;
> +	encoder->possible_crtcs = 1;
> +	encoder->possible_clones = 0;
>  
> -	ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs,
> +	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
>  			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret)
>  		return ret;
> @@ -101,21 +99,19 @@ int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
>  		goto error_encoder_cleanup;
>  	}
>  
> -	ret = drm_mode_connector_attach_encoder(connector, &encoder->base);
> +	ret = drm_mode_connector_attach_encoder(connector, encoder);
>  	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);
>  
>  error_encoder_cleanup:
> -	drm_encoder_cleanup(&encoder->base);
> +	drm_encoder_cleanup(encoder);
>  	return ret;
>  }
> -- 
> 2.15.1
>
Alexey Brodkin Jan. 30, 2018, 4:44 p.m. UTC | #2
Hi Daniel,

On Tue, 2018-01-30 at 10:15 +0100, Daniel Vetter wrote:
> On Wed, Jan 17, 2018 at 03:17:55PM +0100, Daniel Vetter wrote:

> > drm_encoder_slave is the old way to write bridge drivers, for i2c

> > bridges only. It's deprecated, and definitely should not be used in

> > new drivers. This has absolutely nothing to do with the new bridge

> > driver infrastructure implemented by drm_bridge.

> > 

> > What's even strange is that arcpgu doesn't even use any of this, it

> > really only wants a plain normal drm_encoder. Nuke all the surplus

> > real estate.

> > 

> > v2: Actually git add after compile testing ...

> > 

> > v3: Clarify commit message and stop including drm_encoder_slave.h.

> > 

> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > Cc: Alexey Brodkin <abrodkin@synopsys.com>

> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

> 

> I assumed this is ok and applied it to drm-misc-next.

> -Daniel


Not sure but I still cannot see anything arcpgu-related here
https://cgit.freedesktop.org/drm-misc/log/

Am I looking at the wrong place?

-Alexey
Daniel Vetter Jan. 30, 2018, 5:07 p.m. UTC | #3
On Tue, Jan 30, 2018 at 5:44 PM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hi Daniel,
>
> On Tue, 2018-01-30 at 10:15 +0100, Daniel Vetter wrote:
>> On Wed, Jan 17, 2018 at 03:17:55PM +0100, Daniel Vetter wrote:
>> > drm_encoder_slave is the old way to write bridge drivers, for i2c
>> > bridges only. It's deprecated, and definitely should not be used in
>> > new drivers. This has absolutely nothing to do with the new bridge
>> > driver infrastructure implemented by drm_bridge.
>> >
>> > What's even strange is that arcpgu doesn't even use any of this, it
>> > really only wants a plain normal drm_encoder. Nuke all the surplus
>> > real estate.
>> >
>> > v2: Actually git add after compile testing ...
>> >
>> > v3: Clarify commit message and stop including drm_encoder_slave.h.
>> >
>> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> > Cc: Alexey Brodkin <abrodkin@synopsys.com>
>> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>
>> I assumed this is ok and applied it to drm-misc-next.
>> -Daniel
>
> Not sure but I still cannot see anything arcpgu-related here
> https://cgit.freedesktop.org/drm-misc/log/
>
> Am I looking at the wrong place?

Build test script got stuck, pushed for real now.
-Daniel
Alexey Brodkin Jan. 30, 2018, 7:06 p.m. UTC | #4
Hi Daniel,

On Tue, 2018-01-30 at 18:07 +0100, Daniel Vetter wrote:
> On Tue, Jan 30, 2018 at 5:44 PM, Alexey Brodkin

> <Alexey.Brodkin@synopsys.com> wrote:

> > Hi Daniel,

> > 

> > On Tue, 2018-01-30 at 10:15 +0100, Daniel Vetter wrote:

> > > On Wed, Jan 17, 2018 at 03:17:55PM +0100, Daniel Vetter wrote:

> > > > drm_encoder_slave is the old way to write bridge drivers, for i2c

> > > > bridges only. It's deprecated, and definitely should not be used in

> > > > new drivers. This has absolutely nothing to do with the new bridge

> > > > driver infrastructure implemented by drm_bridge.

> > > > 

> > > > What's even strange is that arcpgu doesn't even use any of this, it

> > > > really only wants a plain normal drm_encoder. Nuke all the surplus

> > > > real estate.

> > > > 

> > > > v2: Actually git add after compile testing ...

> > > > 

> > > > v3: Clarify commit message and stop including drm_encoder_slave.h.

> > > > 

> > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > > > Cc: Alexey Brodkin <abrodkin@synopsys.com>

> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

> > > 

> > > I assumed this is ok and applied it to drm-misc-next.

> > > -Daniel

> > 

> > Not sure but I still cannot see anything arcpgu-related here

> > https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_drm-2Dmisc_log_&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656e

> > ViXO7breS55ytWkhpk5R81I&m=ZsFKo3w8gSf8Ul8_sl5RG8BACV1SwqgPV3wf6m4YtVM&s=f9P3-3L_o6CCd_1IQjGPNtAPAOtlO8rj2D2X8T5v_xw&e=

> > 

> > Am I looking at the wrong place?

> 

> Build test script got stuck, pushed for real now.


Thanks!

Gave it a try and it works perfectly fine.
If it still makes any sense...

Acked-by: Alexey Brodkin <abrodkin@synopsys.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c
index 0ce7f398bcff..977dfa55162f 100644
--- a/drivers/gpu/drm/arc/arcpgu_hdmi.c
+++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c
@@ -15,7 +15,8 @@ 
  */
 
 #include <drm/drm_crtc.h>
-#include <drm/drm_encoder_slave.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_device.h>
 
 #include "arcpgu.h"
 
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index bca3a678c955..b8f6f9a5dfbe 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -15,7 +15,6 @@ 
  */
 
 #include <drm/drm_crtc_helper.h>
-#include <drm/drm_encoder_slave.h>
 #include <drm/drm_atomic_helper.h>
 
 #include "arcpgu.h"
@@ -29,7 +28,6 @@ 
 
 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)
@@ -68,7 +66,7 @@  static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = {
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 {
 	struct arcpgu_drm_connector *arcpgu_connector;
-	struct drm_encoder_slave *encoder;
+	struct drm_encoder *encoder;
 	struct drm_connector *connector;
 	int ret;
 
@@ -76,10 +74,10 @@  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 	if (encoder == NULL)
 		return -ENOMEM;
 
-	encoder->base.possible_crtcs = 1;
-	encoder->base.possible_clones = 0;
+	encoder->possible_crtcs = 1;
+	encoder->possible_clones = 0;
 
-	ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs,
+	ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs,
 			       DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret)
 		return ret;
@@ -101,21 +99,19 @@  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np)
 		goto error_encoder_cleanup;
 	}
 
-	ret = drm_mode_connector_attach_encoder(connector, &encoder->base);
+	ret = drm_mode_connector_attach_encoder(connector, encoder);
 	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);
 
 error_encoder_cleanup:
-	drm_encoder_cleanup(&encoder->base);
+	drm_encoder_cleanup(encoder);
 	return ret;
 }