Damian, ping. On 31/01/2019 14:08, Tomi Valkeinen wrote: > Hi, > > On 30/01/2019 13:03, Damian Kos wrote: >> Hello! >> >> This is the series of patches that will add support for the Cadence's DPI/DP >> bridge. Please note that this is a preliminary version of the driver and there >> will be more patches in the future with updates, fixes and improvements. >> Please keep that in mind when looking at FIXME/TODO/XXX comments. >> >> Initially, MHDP driver was developed as a DRM bridge driver and was planed to >> be placed in drivers/gpu/drm/bridge/mhdp.c. However, there was already >> a driver for Cadence's DP controller developed by RockChip, but that driver >> uses the different DRM framework and looks like a part of a bigger system. >> Both controllers (including firmware) are quite different internally >> (MST/FEC/DSC support, link training done by driver, additional commands, IRQ's >> etc.) but they have similar register map, except for Framer/Streamer (which is >> noticeably different), so they appear similar. >> >> The following patches contain: >> - Moving common code to drivers/gpu/drm/bridge/cdns-mhdp-common.* and >> modifying it a bit (mostly new prefixes for functions and data types) so it >> can be used by two, higher level, drivers. >> - Modifying existing RockChip's DP driver to use the common code after changes >> made to it (use the new cdns_mhdp_device structure and new function names). >> - Modifying DRM helpers a bit. Some are required for new driver, some are >> updates from DP 1.2 to 1.3 or 1.4. >> - Adding documentation for device tree bindings. >> - Adding preliminary Cadence DPI/DP bridge driver. >> >> Some of the things that will be added later on include (but are not limited >> to): >> - DSC support >> - FEC support >> - HDCP support > > A few random comments/questions after a quick look at the patches. > > The names of the source files and the kernel Kconfig are only about > "Cadence DP". But the DT bindings is for cdns,mhdp8546, and the > resulting module file is mhdp8546.ko. I think more consistency here > would be good. > > I presume the part number (or family? are there other similar parts with > similar part numbers?) is relevant, so it should be in the Kconfig > option and help text, and probably in the file names too. The module > name should have "cdns" prefix there, similar to the source files and > the cdns-dsi.ko. > > Or maybe the same driver will handle all Cadence DP parts, in which case > generic filenames are fine, but then the resulting kernel module should > also be just "cdns-mhdp.ko". > > I see some audio functions in the code, but it's not mentioned in the DT > bindings. I'm not an audio guy, but the display bridges with audio > support I have seen have had DT bindings for the audio source too. Is > audio supported in the current driver? > > Tomi > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
