Hi Vinod.
Three trivial points below.
The rest looks good.
With these fixed you can add:
Reviewed-by: Sam Ravnborg <s...@ravnborg.org>
Sam
On Wed, Jul 08, 2020 at 04:05:58PM +0530, Vinod Koul wrote:
> Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and
> I2S port as an input and HDMI port as output
>
> Co-developed-by: Bjorn Andersson <bjorn.anders...@linaro.org>
> Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org>
> Co-developed-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> Tested-by: John Stultz <john.stu...@linaro.org>
> Signed-off-by: Vinod Koul <vk...@kernel.org>
> ---
> drivers/gpu/drm/bridge/Kconfig | 13 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/lontium-lt9611.c | 1142 +++++++++++++++++++++++
> 3 files changed, 1156 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/lontium-lt9611.c
>
> +
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_print.h>
In alphabetical order. drm_probe_helper needs to be moved.
> +
> +#define EDID_SEG_SIZE 256
> +#define EDID_LEN 32
> +#define EDID_LOOP 8
> +#define KEY_DDC_ACCS_DONE 0x02
> +#define DDC_NO_ACK 0x50
> +
> +static void lt9611_pcr_setup(struct lt9611 *lt9611, const struct
> drm_display_mode *mode)
> +{
> + const struct reg_sequence reg_cfg[] = {
> + { 0x830b, 0x01 },
> + { 0x830c, 0x10 },
> + { 0x8348, 0x00 },
> + { 0x8349, 0x81 },
> +
> + /* stage 1 */
> + { 0x8321, 0x4a },
> + { 0x8324, 0x71 },
> + { 0x8325, 0x30 },
> + { 0x832a, 0x01 },
> +
> + /* stage 2 */
> + { 0x834a, 0x40 },
> + { 0x831d, 0x10 },
> +
> + /* MK limit */
> + { 0x832d, 0x38 },
> + { 0x8331, 0x08 },
> + };
> + const struct reg_sequence reg_cfg2[] = {
> + { 0x830b, 0x03 },
> + { 0x830c, 0xd0 },
> + { 0x8348, 0x03 },
> + { 0x8349, 0xe0 },
> + { 0x8324, 0x72 },
> + { 0x8325, 0x00 },
> + { 0x832a, 0x01 },
> + { 0x834a, 0x10 },
> + { 0x831d, 0x10 },
> + { 0x8326, 0x37 },
Block above is indented one tab too much.
> +static int lt9611_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> + int ret;
> +
> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> + dev_err(lt9611->dev, "Fix bridge driver to make connector
> optional!");
> + return -EINVAL;
> + }
This should say that the display driver should be fixed.
If a display driver expects this bridge to create the connector
it would not work.
