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:   Mon, 25 Mar 2019 07:18:24 +0100
From:   Jernej Škrabec <jernej.skrabec@...l.net>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     maxime.ripard@...tlin.com, a.hajda@...sung.com, airlied@...ux.ie,
        daniel@...ll.ch, wens@...e.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver

Dne ponedeljek, 25. marec 2019 ob 02:35:31 CET je Laurent Pinchart napisal(a):
> Hi Jernej,
> 
> Thank you for the patch.
> 
> On Sun, Mar 24, 2019 at 10:21:42PM +0100, Jernej Skrabec wrote:
> > DW HDMI controller on some Allwinner SoCs has support for CEC, but due
> > to additional logic put between CEC controller and pins, it doesn't work
> > correctly, at least not with a lot of instrusive changes. Fortunately,
> > it's still possible to bitbang protocol.
> > 
> > For such cases, add a platform option to suppress loading CEC driver. If
> > DW HDMI CEC driver would be loaded, it wouldn't work anyway and only
> > cause a confusion with multiple /dev entries.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@...l.net>
> > ---
> > 
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> >  include/drm/bridge/dw_hdmi.h              | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > a63e5f0dae56..fdda26f8b056 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -2634,7 +2634,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
> > 
> >  		hdmi->audio = platform_device_register_full(&pdevinfo);
> >  	
> >  	}
> > 
> > -	if (config0 & HDMI_CONFIG0_CEC) {
> > +	if (!plat_data->is_cec_unusable && (config0 & HDMI_CONFIG0_CEC)) {
> > 
> >  		cec.hdmi = hdmi;
> >  		cec.ops = &dw_hdmi_cec_ops;
> >  		cec.irq = irq;
> > 
> > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> > index 66e70770cce5..764b8bcfa62c 100644
> > --- a/include/drm/bridge/dw_hdmi.h
> > +++ b/include/drm/bridge/dw_hdmi.h
> > @@ -144,6 +144,8 @@ struct dw_hdmi_plat_data {
> > 
> >  	int (*configure_phy)(struct dw_hdmi *hdmi,
> >  	
> >  			     const struct dw_hdmi_plat_data *pdata,
> >  			     unsigned long mpixelclock);
> > 
> > +
> > +	unsigned int is_cec_unusable : 1;
> 
> Strictly speaking your CEC controller isn't unusable, it's just a bit
> difficult to use it according to your commit message. Would disable_cec
> be a more appropriate field name ? And how difficult would it be to
> support the hardware CEC controller, would that result in changes that
> could be useful to other vendors too ?

Ok, maybe naming of the variable is a bit harsh. Currently I don't know any 
other hardware with the same issue. While I don't know CEC protocol or DW HDMI 
CEC controller very well, I think it would be difficult to implement change of 
direction during ACK transmission. At that time, packet is not fully received 
or transmitted, so I doubt we could use any interrupt for that. In any case, I 
don't think it's worth the hassle and we should just use proven software 
implementation.

Early attempt on fixing this on kernel provided by Allwinner resulted in very 
ugly code and I'm not sure it works 100% (driver based on i.MX6 one):
https://github.com/jernejsk/OpenELEC-OPi2/blob/openelec-7.0/projects/H3/
patches/linux/linux-008-add-hw-hdmi-cec-support.patch#L1321

> 
> >  };
> >  
> >  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,




Powered by blists - more mailing lists