linux-kernel - Re: [LINUX PATCH 2/2] drm: xlnx: dsi: driver for Xilinx DSI Tx subsystem

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, 13 Jun 2022 00:28:03 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Sam Ravnborg <sam@...nborg.org>
Cc:     Venkateshwar Rao Gannavarapu 
        <venkateshwar.rao.gannavarapu@...inx.com>, airlied@...ux.ie,
        vgannava@...inx.com, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [LINUX PATCH 2/2] drm: xlnx: dsi: driver for Xilinx DSI Tx
 subsystem

Hi Sam,

One more comment.

On Fri, May 13, 2022 at 01:05:06PM +0200, Sam Ravnborg wrote:
> On Thu, May 12, 2022 at 07:23:13PM +0530, Venkateshwar Rao Gannavarapu wrote:
> > The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
> > data from AXI-4 stream interface.
> > 
> > It supports upto 4 lanes, optional register interface for the DPHY
> > and multiple RGB color formats.
> > This is a MIPI-DSI host driver and provides DSI bus for panels.
> > This driver also helps to communicate with its panel using panel
> > framework.
> 
> Thanks for submitting this driver. I have added a few comments in the
> following that I hope you will find useful to improve the driver.
> 
> > Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@...inx.com>
> > ---
> >  drivers/gpu/drm/xlnx/Kconfig    |  14 ++
> >  drivers/gpu/drm/xlnx/Makefile   |   1 +
> >  drivers/gpu/drm/xlnx/xlnx_dsi.c | 456 ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 471 insertions(+)
> >  create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c

[snip]

> > diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> > new file mode 100644
> > index 0000000..a5291f3
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c

[snip]

> > +static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = {
> > +	.mode_set	= xlnx_dsi_bridge_mode_set,
> 
> From the documentation of the mode_set operation:
>  * This is deprecated, do not use!
>  * New drivers shall set their mode in the
>  * &drm_bridge_funcs.atomic_enable operation.
> 
> Please adjust accordingly.
> 
> > +	.atomic_enable	= xlnx_dsi_bridge_enable,
> > +	.atomic_disable	= xlnx_dsi_bridge_disable,
> > +	.attach		= xlnx_dsi_bridge_attach,
> > +	.detach		= xlnx_dsi_bridge_detach,
> > +};
> 
> For a new bridge please implement all the mandatory atomic operations.
> 
> You will need at least:
> 	.atomic_get_output_bus_fmts = xlnx_dsi_bridge_get_output_bus_fmts,

As this DSI encoder will never be the last bridge in the chain (there
should always be a panel or another bridge afterwards), I think this
function can be skipped.

> 	.atomic_get_input_bus_fmts = xlnx_dsi_bridge_get_input_bus_fmts,
> 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> 	.atomic_reset = drm_atomic_helper_bridge_reset,
> };

[snip]

-- 
Regards,

Laurent Pinchart