Thank you Sam for your review, I'll send now V2 implementing all your remarks. Best regards, Bogdan > > Hi Bogdan > > On Mon, Oct 05, 2020 at 05:12:08PM +0300, Bogdan Togorean wrote: > > From: Lars-Peter Clausen <lars@xxxxxxxxxx> > > > > The AXI HDMI HDL driver is the driver for the HDL graphics core which is > > used on various FPGA designs. It's mostly used to interface with the > > ADV7511 driver on some Zynq boards (e.g. ZC702 & ZedBoard). > > > > Link: https://wiki.analog.com/resources/tools-software/linux-drivers/drm/hdl- > axi-hdmi > > Link: https://wiki.analog.com/resources/fpga/docs/axi_hdmi_tx > > Thanks for submitting the driver - a few high level comments after > browsing the driver: > > - Use drmm_mode_config_init() to utilize new cleanup > - Look at other uses of drm_driver - there is macros that makes this > much simpler / smaller > - Use devm_drm_dev_alloc() to allocate axi_hdmi_tx_private > To do so embed drm_device in axi_hdmi_tx_private - which is the way to > do it today > - Do not use ddev->dev_private, it is deprecated > - Use dev_err_probe() when you risk to see a PROBE_DEFER > - In all include blocks sort the include alphabetically > - Use the new interface to drm_bridge_attach() - where display driver > creates the connector > - See if the Kconfig selects can be trimmed - the framebuffer releated > selects looks wrong (others get it wrong too) > - Check if you can use the simple encoder - see > drm_simple_encoder_init() > > If this is a simple one plane, one crtc display driver then it should > use the drm_simple_* support. Or the changelog should explain why not. > > We want the drivers as simple as we can - and they shall use as much of > the helper infrastructure as they can. > > We continue to develop the helper infrastructure so it is expected that > there is some lacking behind as is the case here. > > Sam >
