Linux-OMAP Archive on lore.kernel.org
 help / color / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Hans Verkuil <hverkuil-cisco@xs4all.nl>, linux-media@vger.kernel.org
Cc: Tony Lindgren <tony@atomide.com>, Sekhar Nori <nsekhar@ti.com>,
	dri-devel@lists.freedesktop.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-omap@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCHv2 1/6] drm: drm_bridge: add connector_attach/detach bridge ops
Date: Fri, 16 Apr 2021 10:46:08 +0300
Message-ID: <3ba8c7c3-2e86-964d-2e5b-5cdd805def5c@ideasonboard.com> (raw)
In-Reply-To: <20210302162403.983585-2-hverkuil-cisco@xs4all.nl>

Hi Hans,

On 02/03/2021 18:23, Hans Verkuil wrote:
> Add bridge connector_attach/detach ops. These ops are called when a
> bridge is attached or detached to a drm_connector. These ops can be
> used to register and unregister an HDMI CEC adapter for a bridge that
> supports CEC.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>   drivers/gpu/drm/drm_bridge_connector.c |  9 +++++++++
>   include/drm/drm_bridge.h               | 27 ++++++++++++++++++++++++++
>   2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
> index 791379816837..07db71d4f5b3 100644
> --- a/drivers/gpu/drm/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/drm_bridge_connector.c
> @@ -203,6 +203,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector)
>   {
>   	struct drm_bridge_connector *bridge_connector =
>   		to_drm_bridge_connector(connector);
> +	struct drm_bridge *bridge;
> +
> +	drm_for_each_bridge_in_chain(bridge_connector->encoder, bridge)
> +		if (bridge->funcs->connector_detach)
> +			bridge->funcs->connector_detach(bridge, connector);
>   
>   	if (bridge_connector->bridge_hpd) {
>   		struct drm_bridge *hpd = bridge_connector->bridge_hpd;
> @@ -375,6 +380,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>   		connector->polled = DRM_CONNECTOR_POLL_CONNECT
>   				  | DRM_CONNECTOR_POLL_DISCONNECT;
>   
> +	drm_for_each_bridge_in_chain(encoder, bridge)
> +		if (bridge->funcs->connector_attach)
> +			bridge->funcs->connector_attach(bridge, connector);
> +
>   	return connector;
>   }
>   EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 2195daa289d2..3320a6ebd253 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -629,6 +629,33 @@ struct drm_bridge_funcs {
>   	 * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops.
>   	 */
>   	void (*hpd_disable)(struct drm_bridge *bridge);
> +
> +	/**
> +	 * @connector_attach:
> +	 *
> +	 * This callback is invoked whenever our bridge is being attached to a
> +	 * &drm_connector. This is where an HDMI CEC adapter can be registered.
> +	 * Note that this callback expects that this op always succeeds. Since
> +	 * HDMI CEC support is an optional feature, any failure to register a
> +	 * CEC adapter must be ignored since video output will still work
> +	 * without CEC.
> +	 *

Even if CEC support is optional, the callback itself is generic. 
Wouldn't it be better to make this function return an error, and for 
CEC, just return 0 if CEC won't get registered correctly?

Also, I personally like things to fail if something doesn't go right, 
instead of continuing, if that thing is never supposed to happen in 
normal situations. E.g. if CEC registration fails because we're out of 
memory, I think the op should fail too.

  Tomi

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-02 16:23 [PATCHv2 0/6] drm/omap: hdmi: improve hdmi4 CEC, add CEC for hdmi5 Hans Verkuil
2021-03-02 16:23 ` [PATCHv2 1/6] drm: drm_bridge: add connector_attach/detach bridge ops Hans Verkuil
2021-04-16  7:46   ` Tomi Valkeinen [this message]
2021-04-28 12:10     ` Hans Verkuil
2021-03-02 16:23 ` [PATCHv2 2/6] drm/omapdrm/dss/hdmi4: switch to the connector " Hans Verkuil
2021-04-16  7:55   ` Tomi Valkeinen
2021-03-02 16:24 ` [PATCHv2 3/6] drm/omapdrm/dss/hdmi4: simplify CEC Phys Addr handling Hans Verkuil
2021-03-02 16:24 ` [PATCHv2 4/6] dt-bindings: display: ti: ti,omap5-dss.txt: add cec clock Hans Verkuil
2021-03-03  7:40   ` Tomi Valkeinen
2021-03-08 18:59   ` [PATCHv2 4/6] dt-bindings: display: ti: ti, omap5-dss.txt: " Rob Herring
2021-03-02 16:24 ` [PATCHv2 5/6] dra7.dtsi/omap5.dtsi: " Hans Verkuil
2021-03-03  7:44   ` Tomi Valkeinen
2021-03-02 16:24 ` [PATCHv2 6/6] drm/omapdrm/dss/hdmi5: add CEC support Hans Verkuil
2021-03-03  7:47   ` Tomi Valkeinen
2021-03-03  7:51     ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ba8c7c3-2e86-964d-2e5b-5cdd805def5c@ideasonboard.com \
    --to=tomi.valkeinen@ideasonboard.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=nsekhar@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-OMAP Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-omap/0 linux-omap/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-omap linux-omap/ https://lore.kernel.org/linux-omap \
		linux-omap@vger.kernel.org
	public-inbox-index linux-omap

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-omap


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git