linux-kernel - Re: [PATCH 3/3] media: rcar-vin: Fix RAW10

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]

Message-ID: <c037c38d-1056-4ced-b411-c3b8f04162f2@ideasonboard.com>
Date: Thu, 3 Apr 2025 11:18:46 +0300
From: Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>
To: Niklas Söderlund <niklas.soderlund@...natech.se>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-media@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
 Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Subject: Re: [PATCH 3/3] media: rcar-vin: Fix RAW10

Hi,

On 01/04/2025 00:11, Niklas Söderlund wrote:
> Hi Tomi,
> 
> Thanks for your work.
> 
> On 2025-03-24 13:48:54 +0200, Tomi Valkeinen wrote:
>> Fix the following to get RAW10 formats working:
>>
>> In rvin_formats, the bpp is set to 4 for RAW10. As VIN unpacks RAW10 to
>> 16-bit containers, the bpp should be 2.
>>
>> Don't set VNDMR_YC_THR to the VNDMR register. The YC_THR is "YC Data
>> Through Mode", used for YUV formats and should not be set for RAW10.
>>
>> Fix the check related to the RGB666 format and CSI-2 mode. The
>> VNMC_INF_RGB666 define is the same as used for RAW10 on Gen4, and RAW10
>> is allowed on CSI-2 (whereas RGB666 is not allowed on Gen3 on CSI-2).
>> This feels a bit hacky, though, and the formats should really have been
>> verified already earlier.
> 
> I agree, it feels hacky. I would rather just remove the while switch
> then try to "fix" it by extending it more. When testing this series I
> needed a similar fix for VNMC_INF_RAW8 check below to get it to work on
> Gen4.

Also, I think it's fine to be a bit hacky to have a small fix for a 
feature already in upstream, instead of doing a bigger restructuring to 
fix it.

All this should be cleaned up, but in my opinion it's better to do that 
on top of a working upstream.

  Tomi

>>
>> Fixes: 1b7e7240eaf3 ("media: rcar-vin: Add support for RAW10")
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>
>> ---
>>   drivers/media/platform/renesas/rcar-vin/rcar-dma.c  | 9 +++++++--
>>   drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c | 8 ++++----
>>   2 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
>> index 53046614f7a1..f8394be8a922 100644
>> --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
>> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
>> @@ -811,12 +811,17 @@ static int rvin_setup(struct rvin_dev *vin)
>>   		case VNMC_INF_YUV8_BT656:
>>   		case VNMC_INF_YUV10_BT656:
>>   		case VNMC_INF_YUV16:
>> -		case VNMC_INF_RGB666:
>>   			if (vin->is_csi) {
>>   				vin_err(vin, "Invalid setting in MIPI CSI2\n");
>>   				return -EINVAL;
>>   			}
>>   			break;
>> +		case VNMC_INF_RGB666:
>> +			if (vin->info->model == RCAR_GEN3 && vin->is_csi) {
>> +				vin_err(vin, "Invalid setting in MIPI CSI2\n");
>> +				return -EINVAL;
>> +			}
>> +			break;
>>   		case VNMC_INF_RAW8:
>>   			if (!vin->is_csi) {
>>   				vin_err(vin, "Invalid setting in Digital Pins\n");
>> @@ -913,7 +918,7 @@ static int rvin_setup(struct rvin_dev *vin)
>>   	case V4L2_PIX_FMT_SGBRG10:
>>   	case V4L2_PIX_FMT_SGRBG10:
>>   	case V4L2_PIX_FMT_SRGGB10:
>> -		dmr = VNDMR_RMODE_RAW10 | VNDMR_YC_THR;
>> +		dmr = VNDMR_RMODE_RAW10;
>>   		break;
>>   	default:
>>   		vin_err(vin, "Invalid pixelformat (0x%x)\n",
>> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
>> index 756fdfdbce61..65da8d513b52 100644
>> --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
>> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
>> @@ -88,19 +88,19 @@ static const struct rvin_video_format rvin_formats[] = {
>>   	},
>>   	{
>>   		.fourcc			= V4L2_PIX_FMT_SBGGR10,
>> -		.bpp			= 4,
>> +		.bpp			= 2,
>>   	},
>>   	{
>>   		.fourcc			= V4L2_PIX_FMT_SGBRG10,
>> -		.bpp			= 4,
>> +		.bpp			= 2,
>>   	},
>>   	{
>>   		.fourcc			= V4L2_PIX_FMT_SGRBG10,
>> -		.bpp			= 4,
>> +		.bpp			= 2,
>>   	},
>>   	{
>>   		.fourcc			= V4L2_PIX_FMT_SRGGB10,
>> -		.bpp			= 4,
>> +		.bpp			= 2,
>>   	},
>>   };
>>   
>>
>> -- 
>> 2.43.0
>>
>