[RFC] drm: rcar-du: Remove i2c slave encoder interface for hdmi encoder

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Feb 25 23:04:19 UTC 2016


Hi Archit,

Thank you for the patch, I finally got around to testing it. Sorry for the 
delay.

On Saturday 09 January 2016 22:22:26 Archit Taneja wrote:
> The hdmi output in rcar-du uses the i2c slave encoder interface to link
> to the adv7511 encoder chip. The kms driver creates encoder and connector
> entities that internally uses the drm_encoder_slave_funcs ops provided by
> the slave encoder driver.
> 
> Change the driver such that it expects a bridge entity instead of a slave
> encoder. The hdmi connector code isn't needed anymore as we expect the
> adv7511 bridge driver to create/manage the connector.
> 
> Note that the kms driver still expects a connector node for hdmi to be
> present in DT. This node has no connection to the connector created
> by the bridge driver.
> 
> Compile tested only.

And doesn't apply anymore, but that's my fault for not reviewing it fast 
enough, so I've rebased it (as well as your "[PATCH] drm: i2c: adv7511: 
Convert to drm_bridge" patch). I've pushed the result to

	git://linuxtv.org/pinchartl/media.git drm/du/bridge

The rebase wasn't difficult, I don't have much to say about that. What I must 
report, however, is that it doesn't work :-/ I receive the following kernel 
warning:

[    2.165569] ------------[ cut here ]------------
[    2.170205] WARNING: CPU: 2 PID: 6 at 
/home/laurent/src/iob/renesas/linux/lib/kobject.c:244 
kobject_add_internal+0x134/0x364()
[    2.181627] kobject_add_internal failed for card0-HDMI-A-1 (error: -2 
parent: card0)
[    2.189394] Modules linked in:
[    2.192466] CPU: 2 PID: 6 Comm: kworker/u16:0 Not tainted 4.5.0-rc5-00677-
g6e2d7cd77473 #649
[    2.200914] Hardware name: Generic R8A7790 (Flattened Device Tree)
[    2.207103] Workqueue: deferwq deferred_probe_work_func
[    2.212341] Backtrace: 
[    2.214816] [<c0014c98>] (dump_backtrace) from [<c0014fb0>] 
(show_stack+0x20/0x24)
[    2.222396]  r6:c0591910 r5:00000000 r4:60000013 r3:ea0e4000
[    2.228104] [<c0014f90>] (show_stack) from [<c01f259c>] 
(dump_stack+0x8c/0xac)
[    2.235339] [<c01f2510>] (dump_stack) from [<c002adb8>] 
(warn_slowpath_common+0x88/0xc4)
[    2.243438]  r5:000000f4 r4:ea0e5b48
[    2.247038] [<c002ad30>] (warn_slowpath_common) from [<c002aeb0>] 
(warn_slowpath_fmt+0x40/0x48)
[    2.255746]  r8:ea38ac08 r7:ea38ac00 r6:fffffffe r5:00000000 r4:ea38aa08
[    2.262503] [<c002ae74>] (warn_slowpath_fmt) from [<c01f54f0>] 
(kobject_add_internal+0x134/0x364)
[    2.271385]  r3:c04715c8 r2:c0591bf4
[    2.274986] [<c01f53bc>] (kobject_add_internal) from [<c01f5904>] 
(kobject_add+0x54/0x9c)
[    2.283173]  r8:00000000 r7:ea38ac00 r6:ea38ac08 r5:00000000 r4:ea38aa08
[    2.289938] [<c01f58b4>] (kobject_add) from [<c02c6d98>] 
(device_add+0xe0/0x558)
[    2.297343]  r3:00000000 r2:00000000
[    2.300938]  r6:ea38aa08 r5:00000000 r4:ea38aa00
[    2.305592] [<c02c6cb8>] (device_add) from [<c02c72c0>] 
(device_create_groups_vargs+0xb0/0xe0)
[    2.314213]  r10:e87af86c r9:e87c826c r8:00000000 r7:ea38ac00 r6:e9a1f960 
r5:e9a18a40
[    2.322101]  r4:ea38aa00
[    2.324651] [<c02c7210>] (device_create_groups_vargs) from [<c02c7324>] 
(device_create_with_groups+0x34/0x3c)
[    2.334575]  r8:e9a1f940 r7:c069ccec r6:c08d4a54 r5:ea388c00 r4:c069ccec 
r3:e9a1f960
[    2.342385] [<c02c72f0>] (device_create_with_groups) from [<c02a6cec>] 
(drm_sysfs_connector_add+0x6c/0xe0)
[    2.352048]  r4:e9a1f960
[    2.354600] [<c02a6c80>] (drm_sysfs_connector_add) from [<c02aa1f0>] 
(drm_connector_register+0x4c/0x7c)
[    2.364002]  r7:ea388c00 r6:ea388d70 r5:e9a1f974 r4:e9a1f960
[    2.369709] [<c02aa1a4>] (drm_connector_register) from [<c02c3270>] 
(adv7511_bridge_attach+0x5c/0xa8)
[    2.378938]  r7:e99fa550 r6:00000000 r5:e9a1f960 r4:e9a1f940
[    2.384643] [<c02c3214>] (adv7511_bridge_attach) from [<c02bd090>] 
(drm_bridge_attach+0x48/0x64)
[    2.393438]  r6:00000000 r5:e9814010 r4:e99f1c90 r3:c02c3214
[    2.399142] [<c02bd048>] (drm_bridge_attach) from [<c02c22a8>] 
(rcar_du_hdmienc_init+0x94/0x108)
[    2.407942] [<c02c2214>] (rcar_du_hdmienc_init) from [<c02bf9ec>] 
(rcar_du_encoder_init+0x14c/0x19c)
[    2.417084]  r8:00000002 r7:00000004 r6:00000002 r5:e9814010 r4:e99f1c90
[    2.423840] [<c02bf8a0>] (rcar_du_encoder_init) from [<c02c0a70>] 
(rcar_du_modeset_init+0x4a4/0x574)
[    2.432982]  r10:00000002 r8:00000001 r7:c04b7e10 r6:e87af86c r5:e87b88b4 
r4:e9814010
[    2.440873] [<c02c05cc>] (rcar_du_modeset_init) from [<c02bf600>] 
(rcar_du_probe+0xfc/0x230)
[    2.449319]  r10:00000001 r9:00000000 r8:00000000 r7:ea315c10 r6:ea388c00 
r5:ea315c00
[    2.457206]  r4:e9814010
[    2.459756] [<c02bf504>] (rcar_du_probe) from [<c02cc360>] 
(platform_drv_probe+0x60/0xc0)
[    2.467943]  r10:00000005 r8:c069d31c r7:c069d31c r6:fffffdfb r5:ea315c10 
r4:fffffffe
[    2.475833] [<c02cc300>] (platform_drv_probe) from [<c02ca1d0>] 
(driver_probe_device+0x224/0x440)
[    2.484715]  r7:c06b4bd0 r6:c08d4b04 r5:c08d4afc r4:ea315c10
[    2.490419] [<c02c9fac>] (driver_probe_device) from [<c02ca6bc>] 
(__device_attach_driver+0x90/0x9c)
[    2.499474]  r10:00000000 r9:00000000 r8:ea2f9f00 r7:00000001 r6:ea315c10 
r5:ea0e5e70
[    2.507360]  r4:c069d31c
[    2.509907] [<c02ca62c>] (__device_attach_driver) from [<c02c7fc4>] 
(bus_for_each_drv+0x54/0x9c)
[    2.518702]  r6:c02ca62c r5:ea0e5e70 r4:00000000 r3:ea0e4000
[    2.524406] [<c02c7f70>] (bus_for_each_drv) from [<c02c9e94>] 
(__device_attach+0xac/0x140)
[    2.532679]  r6:c069d6d8 r5:ea315c44 r4:ea315c10
[    2.537331] [<c02c9de8>] (__device_attach) from [<c02ca6e4>] 
(device_initial_probe+0x1c/0x20)
[    2.545865]  r7:c06b4bd0 r6:c069d6d8 r5:ea315c10 r4:ea315c10
[    2.551568] [<c02ca6c8>] (device_initial_probe) from [<c02c91dc>] 
(bus_probe_device+0x94/0x9c)
[    2.560192] [<c02c9148>] (bus_probe_device) from [<c02c9778>] 
(deferred_probe_work_func+0x80/0xcc)
[    2.569160]  r6:c069d680 r5:c069d660 r4:ea315c10 r3:00000000
[    2.574869] [<c02c96f8>] (deferred_probe_work_func) from [<c0042cac>] 
(process_one_work+0x170/0x494)
[    2.584011]  r7:c06b0a38 r6:ea089400 r5:c069d694 r4:ea03e000
[    2.589716] [<c0042b3c>] (process_one_work) from [<c0043054>] 
(worker_thread+0x3c/0x55c)
[    2.597815]  r10:ea089400 r9:c066c100 r8:00000088 r7:ea03e018 r6:ea089418 
r5:ea089400
[    2.605701]  r4:ea03e000
[    2.608253] [<c0043018>] (worker_thread) from [<c00490e0>] 
(kthread+0xf4/0x114)
[    2.615571]  r10:00000000 r9:00000000 r8:00000000 r7:c0043018 r6:ea03e000 
r5:00000000
[    2.623457]  r4:ea08c0c0
[    2.626006] [<c0048fec>] (kthread) from [<c00119e8>] 
(ret_from_fork+0x14/0x2c)
[    2.633237]  r7:00000000 r6:00000000 r5:c0048fec r4:ea08c0c0
[    2.638961] ---[ end trace a6a190229149dd48 ]---
[    2.643593] [drm:drm_sysfs_connector_add] *ERROR* failed to register 
connector device: -2

This is caused by trying to register a connector to sysfs before the DRM 
device is registered. Since the rcar-du-drm driver has stopped using the race-
prone .load() operation and switched to calling drm_dev_register() explicitly, 
connectors need to be registered after the drm_dev_register() call, and thus 
after attaching to the bridge.

One possible fix here is to remove the drm_connector_register() from the 
adv7511 attach() handler. This gets rid of the warning, and the connector will 
be registered by the rcar-du-drm driver at end of probe along with all the 
other connectors.

This would of course break with drivers that still rely on the .load() 
handler, as the connector would then never be registered. I'm not sure how bad 
an issue this is, given that drivers are supposed to move away from .load(). 
It could thus be argued that needing the adv7511 bridge driver would be a good 
incentive.

Another solution is to register the connector in the DRM driver instead of the 
bridge driver. I might have mentioned that already ;-) I still believe that 
would be a good idea.

I've removed the drm_connector_register() call from the adv7511 driver for 
testing purpose. The warning goes away, but unfortunately the HDMI output 
remains disabled. I'll try to investigate why.

> Signed-off-by: Archit Taneja <architt at codeaurora.org>
> ---
>  drivers/gpu/drm/rcar-du/Makefile          |   3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c |   3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.h |   7 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c | 126 ---------------------------
>  drivers/gpu/drm/rcar-du/rcar_du_hdmicon.h |  31 --------
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c |  60 ++++----------
>  6 files changed, 20 insertions(+), 210 deletions(-)
>  delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c
>  delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmicon.h
> 
> diff --git a/drivers/gpu/drm/rcar-du/Makefile
> b/drivers/gpu/drm/rcar-du/Makefile index 05de1c4..e8cf451 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -7,8 +7,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>  		 rcar_du_plane.o \
>  		 rcar_du_vgacon.o
> 
> -rcar-du-drm-$(CONFIG_DRM_RCAR_HDMI)	+= rcar_du_hdmicon.o \
> -					   rcar_du_hdmienc.o
> +rcar-du-drm-$(CONFIG_DRM_RCAR_HDMI)	+= rcar_du_hdmienc.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_lvdsenc.o
> 
>  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index d0ae1e8..ea00e33 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -19,7 +19,6 @@
> 
>  #include "rcar_du_drv.h"
>  #include "rcar_du_encoder.h"
> -#include "rcar_du_hdmicon.h"
>  #include "rcar_du_hdmienc.h"
>  #include "rcar_du_kms.h"
>  #include "rcar_du_lvdscon.h"
> @@ -190,7 +189,7 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  		break;
> 
>  	case DRM_MODE_ENCODER_TMDS:
> -		ret = rcar_du_hdmi_connector_init(rcdu, renc);
> +		/* connector managed by the bridge driver */
>  		break;
> 
>  	default:
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h index 719b6f2a..dde523a 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> @@ -15,7 +15,6 @@
>  #define __RCAR_DU_ENCODER_H__
> 
>  #include <drm/drm_crtc.h>
> -#include <drm/drm_encoder_slave.h>
> 
>  struct rcar_du_device;
>  struct rcar_du_hdmienc;
> @@ -30,16 +29,16 @@ enum rcar_du_encoder_type {
>  };
> 
>  struct rcar_du_encoder {
> -	struct drm_encoder_slave slave;
> +	struct drm_encoder base;
>  	enum rcar_du_output output;
>  	struct rcar_du_hdmienc *hdmi;
>  	struct rcar_du_lvdsenc *lvds;
>  };
> 
>  #define to_rcar_encoder(e) \
> -	container_of(e, struct rcar_du_encoder, slave.base)
> +	container_of(e, struct rcar_du_encoder, base)
> 
> -#define rcar_encoder_to_drm_encoder(e)	(&(e)->slave.base)
> +#define rcar_encoder_to_drm_encoder(e)	(&(e)->base)
> 
>  struct rcar_du_connector {
>  	struct drm_connector connector;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c
> b/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c deleted file mode 100644
> index 96f2eb4..0000000
> --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c
> +++ /dev/null
> @@ -1,126 +0,0 @@
> -/*
> - * R-Car Display Unit HDMI Connector
> - *
> - * Copyright (C) 2014 Renesas Electronics Corporation
> - *
> - * Contact: Laurent Pinchart (laurent.pinchart at ideasonboard.com)
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - */
> -
> -#include <drm/drmP.h>
> -#include <drm/drm_atomic_helper.h>
> -#include <drm/drm_crtc.h>
> -#include <drm/drm_crtc_helper.h>
> -#include <drm/drm_encoder_slave.h>
> -
> -#include "rcar_du_drv.h"
> -#include "rcar_du_encoder.h"
> -#include "rcar_du_hdmicon.h"
> -#include "rcar_du_kms.h"
> -
> -#define to_slave_funcs(e)	(to_rcar_encoder(e)->slave.slave_funcs)
> -
> -static int rcar_du_hdmi_connector_get_modes(struct drm_connector
> *connector) -{
> -	struct rcar_du_connector *con = to_rcar_connector(connector);
> -	struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(con->encoder);
> -	struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
> -
> -	if (sfuncs->get_modes == NULL)
> -		return 0;
> -
> -	return sfuncs->get_modes(encoder, connector);
> -}
> -
> -static int rcar_du_hdmi_connector_mode_valid(struct drm_connector
> *connector, -					     struct drm_display_mode *mode)
> -{
> -	struct rcar_du_connector *con = to_rcar_connector(connector);
> -	struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(con->encoder);
> -	struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
> -
> -	if (sfuncs->mode_valid == NULL)
> -		return MODE_OK;
> -
> -	return sfuncs->mode_valid(encoder, mode);
> -}
> -
> -static const struct drm_connector_helper_funcs connector_helper_funcs = {
> -	.get_modes = rcar_du_hdmi_connector_get_modes,
> -	.mode_valid = rcar_du_hdmi_connector_mode_valid,
> -	.best_encoder = rcar_du_connector_best_encoder,
> -};
> -
> -static void rcar_du_hdmi_connector_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_unregister(connector);
> -	drm_connector_cleanup(connector);
> -}
> -
> -static enum drm_connector_status
> -rcar_du_hdmi_connector_detect(struct drm_connector *connector, bool force)
> -{
> -	struct rcar_du_connector *con = to_rcar_connector(connector);
> -	struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(con->encoder);
> -	struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
> -
> -	if (sfuncs->detect == NULL)
> -		return connector_status_unknown;
> -
> -	return sfuncs->detect(encoder, connector);
> -}
> -
> -static const struct drm_connector_funcs connector_funcs = {
> -	.dpms = drm_atomic_helper_connector_dpms,
> -	.reset = drm_atomic_helper_connector_reset,
> -	.detect = rcar_du_hdmi_connector_detect,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = rcar_du_hdmi_connector_destroy,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
> -int rcar_du_hdmi_connector_init(struct rcar_du_device *rcdu,
> -				struct rcar_du_encoder *renc)
> -{
> -	struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(renc);
> -	struct rcar_du_connector *rcon;
> -	struct drm_connector *connector;
> -	int ret;
> -
> -	rcon = devm_kzalloc(rcdu->dev, sizeof(*rcon), GFP_KERNEL);
> -	if (rcon == NULL)
> -		return -ENOMEM;
> -
> -	connector = &rcon->connector;
> -	connector->display_info.width_mm = 0;
> -	connector->display_info.height_mm = 0;
> -	connector->interlace_allowed = true;
> -	connector->polled = DRM_CONNECTOR_POLL_HPD;
> -
> -	ret = drm_connector_init(rcdu->ddev, connector, &connector_funcs,
> -				 DRM_MODE_CONNECTOR_HDMIA);
> -	if (ret < 0)
> -		return ret;
> -
> -	drm_connector_helper_add(connector, &connector_helper_funcs);
> -	ret = drm_connector_register(connector);
> -	if (ret < 0)
> -		return ret;
> -
> -	connector->dpms = DRM_MODE_DPMS_OFF;
> -	drm_object_property_set_value(&connector->base,
> -		rcdu->ddev->mode_config.dpms_property, DRM_MODE_DPMS_OFF);
> -
> -	ret = drm_mode_connector_attach_encoder(connector, encoder);
> -	if (ret < 0)
> -		return ret;
> -
> -	rcon->encoder = renc;
> -
> -	return 0;
> -}
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.h
> b/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.h deleted file mode 100644
> index 87daa94..0000000
> --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/*
> - * R-Car Display Unit HDMI Connector
> - *
> - * Copyright (C) 2014 Renesas Electronics Corporation
> - *
> - * Contact: Laurent Pinchart (laurent.pinchart at ideasonboard.com)
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - */
> -
> -#ifndef __RCAR_DU_HDMICON_H__
> -#define __RCAR_DU_HDMICON_H__
> -
> -struct rcar_du_device;
> -struct rcar_du_encoder;
> -
> -#if IS_ENABLED(CONFIG_DRM_RCAR_HDMI)
> -int rcar_du_hdmi_connector_init(struct rcar_du_device *rcdu,
> -				struct rcar_du_encoder *renc);
> -#else
> -static inline int rcar_du_hdmi_connector_init(struct rcar_du_device *rcdu,
> -					      struct rcar_du_encoder *renc)
> -{
> -	return -ENOSYS;
> -}
> -#endif
> -
> -#endif /* __RCAR_DU_HDMICON_H__ */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c index 81da841..12ec1d8 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> @@ -16,7 +16,6 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
> -#include <drm/drm_encoder_slave.h>
> 
>  #include "rcar_du_drv.h"
>  #include "rcar_du_encoder.h"
> @@ -25,20 +24,14 @@
> 
>  struct rcar_du_hdmienc {
>  	struct rcar_du_encoder *renc;
> -	struct device *dev;
>  	bool enabled;
>  };
> 
>  #define to_rcar_hdmienc(e)	(to_rcar_encoder(e)->hdmi)
> -#define to_slave_funcs(e)	(to_rcar_encoder(e)->slave.slave_funcs)
> 
>  static void rcar_du_hdmienc_disable(struct drm_encoder *encoder)
>  {
>  	struct rcar_du_hdmienc *hdmienc = to_rcar_hdmienc(encoder);
> -	struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
> -
> -	if (sfuncs->dpms)
> -		sfuncs->dpms(encoder, DRM_MODE_DPMS_OFF);
> 
>  	if (hdmienc->renc->lvds)
>  		rcar_du_lvdsenc_enable(hdmienc->renc->lvds, encoder->crtc,
> @@ -50,15 +43,11 @@ static void rcar_du_hdmienc_disable(struct drm_encoder
> *encoder) static void rcar_du_hdmienc_enable(struct drm_encoder *encoder)
>  {
>  	struct rcar_du_hdmienc *hdmienc = to_rcar_hdmienc(encoder);
> -	struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
> 
>  	if (hdmienc->renc->lvds)
>  		rcar_du_lvdsenc_enable(hdmienc->renc->lvds, encoder->crtc,
>  				       true);
> 
> -	if (sfuncs->dpms)
> -		sfuncs->dpms(encoder, DRM_MODE_DPMS_ON);
> -
>  	hdmienc->enabled = true;
>  }
> 
> @@ -67,9 +56,7 @@ static int rcar_du_hdmienc_atomic_check(struct drm_encoder
> *encoder, struct drm_connector_state *conn_state)
>  {
>  	struct rcar_du_hdmienc *hdmienc = to_rcar_hdmienc(encoder);
> -	struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
>  	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
> -	const struct drm_display_mode *mode = &crtc_state->mode;
> 
>  	/* The internal LVDS encoder has a clock frequency operating range of
>  	 * 30MHz to 150MHz. Clamp the clock accordingly.
> @@ -78,10 +65,7 @@ static int rcar_du_hdmienc_atomic_check(struct
> drm_encoder *encoder, adjusted_mode->clock = clamp(adjusted_mode->clock,
>  					     30000, 150000);
> 
> -	if (sfuncs->mode_fixup == NULL)
> -		return 0;
> -
> -	return sfuncs->mode_fixup(encoder, mode, adjusted_mode) ? 0 : -EINVAL;
> +	return 0;
>  }
> 
>  static void rcar_du_hdmienc_mode_set(struct drm_encoder *encoder,
> @@ -89,10 +73,6 @@ static void rcar_du_hdmienc_mode_set(struct drm_encoder
> *encoder, struct drm_display_mode *adjusted_mode)
>  {
>  	struct rcar_du_hdmienc *hdmienc = to_rcar_hdmienc(encoder);
> -	struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
> -
> -	if (sfuncs->mode_set)
> -		sfuncs->mode_set(encoder, mode, adjusted_mode);
> 
>  	rcar_du_crtc_route_output(encoder->crtc, hdmienc->renc->output);
>  }
> @@ -112,7 +92,6 @@ static void rcar_du_hdmienc_cleanup(struct drm_encoder
> *encoder) rcar_du_hdmienc_disable(encoder);
> 
>  	drm_encoder_cleanup(encoder);
> -	put_device(hdmienc->dev);
>  }
> 
>  static const struct drm_encoder_funcs encoder_funcs = {
> @@ -123,8 +102,7 @@ int rcar_du_hdmienc_init(struct rcar_du_device *rcdu,
>  			 struct rcar_du_encoder *renc, struct device_node *np)
>  {
>  	struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(renc);
> -	struct drm_i2c_encoder_driver *driver;
> -	struct i2c_client *i2c_slave;
> +	struct drm_bridge *bridge;
>  	struct rcar_du_hdmienc *hdmienc;
>  	int ret;
> 
> @@ -132,37 +110,29 @@ int rcar_du_hdmienc_init(struct rcar_du_device *rcdu,
>  	if (hdmienc == NULL)
>  		return -ENOMEM;
> 
> -	/* Locate the slave I2C device and driver. */
> -	i2c_slave = of_find_i2c_device_by_node(np);
> -	if (!i2c_slave || !i2c_get_clientdata(i2c_slave))
> +	/* Locate drm bridge from the hdmi encoder DT node */
> +	bridge = of_drm_find_bridge(np);
> +	if (!bridge)
>  		return -EPROBE_DEFER;
> 
> -	hdmienc->dev = &i2c_slave->dev;
> -
> -	if (hdmienc->dev->driver == NULL) {
> -		ret = -EPROBE_DEFER;
> -		goto error;
> -	}
> -
> -	/* Initialize the slave encoder. */
> -	driver = to_drm_i2c_encoder_driver(to_i2c_driver(hdmienc->dev->driver));
> -	ret = driver->encoder_init(i2c_slave, rcdu->ddev, &renc->slave);
> -	if (ret < 0)
> -		goto error;
> -
>  	ret = drm_encoder_init(rcdu->ddev, encoder, &encoder_funcs,
>  			       DRM_MODE_ENCODER_TMDS);
>  	if (ret < 0)
> -		goto error;
> +		return ret;
> 
>  	drm_encoder_helper_add(encoder, &encoder_helper_funcs);
> 
>  	renc->hdmi = hdmienc;
>  	hdmienc->renc = renc;
> 
> -	return 0;
> +	/* Link drm_bridge to encoder */
> +	bridge->encoder = encoder;
> 
> -error:
> -	put_device(hdmienc->dev);
> -	return ret;
> +	ret = drm_bridge_attach(rcdu->ddev, bridge);
> +	if (ret) {
> +		drm_encoder_cleanup(encoder);
> +		return ret;
> +	}
> +
> +	return 0;
>  }

-- 
Regards,

Laurent Pinchart



More information about the dri-devel mailing list