lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 22 May 2024 05:58:00 +0000
From: Keith Zhao <keith.zhao@...rfivetech.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>, Alex Bee
	<knaerzche@...il.com>
CC: "andrzej.hajda@...el.com" <andrzej.hajda@...el.com>,
	"neil.armstrong@...aro.org" <neil.armstrong@...aro.org>, "rfoss@...nel.org"
	<rfoss@...nel.org>, "jonas@...boo.se" <jonas@...boo.se>,
	"jernej.skrabec@...il.com" <jernej.skrabec@...il.com>,
	"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
	"mripard@...nel.org" <mripard@...nel.org>, "tzimmermann@...e.de"
	<tzimmermann@...e.de>, "airlied@...il.com" <airlied@...il.com>,
	"daniel@...ll.ch" <daniel@...ll.ch>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "hjc@...k-chips.com" <hjc@...k-chips.com>,
	"heiko@...ech.de" <heiko@...ech.de>, "andy.yan@...k-chips.com"
	<andy.yan@...k-chips.com>, Xingyu Wu <xingyu.wu@...rfivetech.com>,
	"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>, Jack Zhu
	<jack.zhu@...rfivetech.com>, Shengyang Chen
	<shengyang.chen@...rfivetech.com>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v4 02/10] drm/bridge: add common api for inno hdmi

Hi Alex, Laurent:

I want to make as few changes as possible on the current basis, and add bridge_fun, 


> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Sent: 2024年5月21日 23:42
> To: Alex Bee <knaerzche@...il.com>
> Cc: Keith Zhao <keith.zhao@...rfivetech.com>; andrzej.hajda@...el.com;
> neil.armstrong@...aro.org; rfoss@...nel.org; jonas@...boo.se;
> jernej.skrabec@...il.com; maarten.lankhorst@...ux.intel.com;
> mripard@...nel.org; tzimmermann@...e.de; airlied@...il.com;
> daniel@...ll.ch; robh@...nel.org; krzk+dt@...nel.org; conor+dt@...nel.org;
> hjc@...k-chips.com; heiko@...ech.de; andy.yan@...k-chips.com; Xingyu Wu
> <xingyu.wu@...rfivetech.com>; p.zabel@...gutronix.de; Jack Zhu
> <jack.zhu@...rfivetech.com>; Shengyang Chen
> <shengyang.chen@...rfivetech.com>; dri-devel@...ts.freedesktop.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org
> Subject: Re: [PATCH v4 02/10] drm/bridge: add common api for inno hdmi
> 
> On Tue, May 21, 2024 at 05:36:43PM +0200, Alex Bee wrote:
> > Hi Keith,
> >
> > thanks a lot for working on this. See some general remarks below
> >
> > Am 21.05.24 um 12:58 schrieb keith:
> > > Add INNO common api so that it can be used by vendor drivers which
> > > implement vendor specific extensions to Innosilicon HDMI.
> > >
> > > Signed-off-by: keith <keith.zhao@...rfivetech.com>
> > > ---
> > >   MAINTAINERS                                   |   2 +
> > >   drivers/gpu/drm/bridge/Kconfig                |   2 +
> > >   drivers/gpu/drm/bridge/Makefile               |   1 +
> > >   drivers/gpu/drm/bridge/innosilicon/Kconfig    |   6 +
> > >   drivers/gpu/drm/bridge/innosilicon/Makefile   |   2 +
> > >   .../gpu/drm/bridge/innosilicon/inno-hdmi.c    | 587
> ++++++++++++++++++
> > >   .../gpu/drm/bridge/innosilicon/inno-hdmi.h    |  97 +++
> > >   include/drm/bridge/inno_hdmi.h                |  69 ++
> > >   8 files changed, 766 insertions(+)
> > >   create mode 100644 drivers/gpu/drm/bridge/innosilicon/Kconfig
> > >   create mode 100644 drivers/gpu/drm/bridge/innosilicon/Makefile
> > >   create mode 100644 drivers/gpu/drm/bridge/innosilicon/inno-hdmi.c
> > >   create mode 100644 drivers/gpu/drm/bridge/innosilicon/inno-hdmi.h
> > >   create mode 100644 include/drm/bridge/inno_hdmi.h
> > >
> > ....
> >
> > > +	drm_encoder_helper_add(encoder, pdata->helper_private);
> > > +
> > > +	hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
> > > +
> > > +	drm_connector_helper_add(&hdmi->connector,
> > > +				 &inno_hdmi_connector_helper_funcs);
> > > +
> > > +	drmm_connector_init(drm, &hdmi->connector,
> > > +			    &inno_hdmi_connector_funcs,
> > > +			    DRM_MODE_CONNECTOR_HDMIA,
> > > +			    hdmi->ddc);
> > > +
> >
> > I really don't want to anticipate bridge maintainer's feedback, but
> > new bridge drivers must not contain connector creation. That must
> > happen somewhere else.
> 
> You're absolutely right :-) Connector creation should be handled by the
> drm_bridge_connector helper. The HDMI bridge driver should focus on the
> HDMI bridge itself.

static int inno_bridge_attach(struct drm_bridge *bridge,
				 enum drm_bridge_attach_flags flags)
{
	struct inno_hdmi *hdmi = bridge_to_inno(bridge);

	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
		DRM_ERROR("Fix bridge driver to make connector optional!");
		return -EINVAL;
	}
	hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;

	drm_connector_helper_add(&hdmi->connector,
				 &inno_hdmi_connector_helper_funcs);

	drmm_connector_init(drm, &hdmi->connector,
			    &inno_hdmi_connector_funcs,
			    DRM_MODE_CONNECTOR_HDMIA,
			    hdmi->ddc);

	drm_connector_attach_encoder(&hdmi->connector, encoder);
}

static const struct drm_bridge_funcs inno_bridge_attach = {
	.attach = inno_bridge_attach,
};

Connector creation is handled by the drm_bridge_funcs ->attach.
Is it ok?

Regards
Keith
> 
> > Also I'm neither seeing any drm_brige struct nor drm_bridge_funcs,
> > which are both essential for a bridge driver. I don't think moving a
> > part of a driver to .../drm/bridge/ makes it a bridge driver.
> >
> > > +	drm_connector_attach_encoder(&hdmi->connector, encoder);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > ....
> >
> 
> --
> Regards,
> 
> Laurent Pinchart

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ