Re: [PATCH v3 6/7] media: rcar-vin: initialize EDID data

[Hans Verkuil <hverkuil@xs4all.nl>]

On 04/14/2016 06:17 PM, Ulrich Hecht wrote:
> Initializes the decoder subdevice with a fixed EDID blob.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 46 +++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index ba2ed4e..5b32105 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -720,6 +720,41 @@ void rvin_v4l2_remove(struct rvin_dev *vin)
>  	video_unregister_device(&vin->vdev);
>  }
>  
> +static u8 edid[256] = {
> +	0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00,
> +	0x48, 0xAE, 0x9C, 0x27, 0x00, 0x00, 0x00, 0x00,
> +	0x19, 0x12, 0x01, 0x03, 0x80, 0x00, 0x00, 0x78,
> +	0x0E, 0x00, 0xB2, 0xA0, 0x57, 0x49, 0x9B, 0x26,
> +	0x10, 0x48, 0x4F, 0x2F, 0xCF, 0x00, 0x31, 0x59,
> +	0x45, 0x59, 0x61, 0x59, 0x81, 0x99, 0x01, 0x01,
> +	0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3A,
> +	0x80, 0x18, 0x71, 0x38, 0x2D, 0x40, 0x58, 0x2C,
> +	0x46, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1E,
> +	0x00, 0x00, 0x00, 0xFD, 0x00, 0x31, 0x55, 0x18,
> +	0x5E, 0x11, 0x00, 0x0A, 0x20, 0x20, 0x20, 0x20,
> +	0x20, 0x20, 0x00, 0x00, 0x00, 0xFC, 0x00, 0x43,
> +	0x20, 0x39, 0x30, 0x0A, 0x0A, 0x0A, 0x0A, 0x0A,
> +	0x0A, 0x0A, 0x0A, 0x0A, 0x00, 0x00, 0x00, 0x10,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x68,
> +	0x02, 0x03, 0x1a, 0xc0, 0x48, 0xa2, 0x10, 0x04,
> +	0x02, 0x01, 0x21, 0x14, 0x13, 0x23, 0x09, 0x07,
> +	0x07, 0x65, 0x03, 0x0c, 0x00, 0x10, 0x00, 0xe2,
> +	0x00, 0x2a, 0x01, 0x1d, 0x00, 0x80, 0x51, 0xd0,
> +	0x1c, 0x20, 0x40, 0x80, 0x35, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x1e, 0x8c, 0x0a, 0xd0, 0x8a,
> +	0x20, 0xe0, 0x2d, 0x10, 0x10, 0x3e, 0x96, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd7
> +};

Where does this EDID come from? I'm just wondering if it has been
adjusted for the capabilities of the adv.

BTW, it is useful if userspace can read the EDID via VIDIOC_G_EDID.

In general I am of two minds whether the EDID should be set in the driver
or whether it should be left to userspace. The EDID contains vendor IDs
and things like that, which are generally better left to userspace for
embedded systems.

Note that the v4l2-ctl utility has support to fill the edid to a standard HDMI
EDID. See v4l2-ctl --help-edid.

My feeling is that it is better to add G/S_EDID support to the r-car driver
and not initialize the EDID at all.

Regards,

	Hans

> +
>  int rvin_v4l2_probe(struct rvin_dev *vin)
>  {
>  	struct v4l2_subdev_format fmt = {
> @@ -821,5 +856,16 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
>  	v4l2_info(&vin->v4l2_dev, "Device registered as %s\n",
>  		  video_device_node_name(&vin->vdev));
>  
> +	{
> +		struct v4l2_subdev_edid rvin_edid = {
> +			.pad = 0,
> +			.start_block = 0,
> +			.blocks = 2,
> +			.edid = edid,
> +		};
> +		v4l2_subdev_call(sd, pad, set_edid,
> +				&rvin_edid);
> +	}
> +
>  	return ret;
>  }
>