Re: [PATCH 7/8] media: atomisp: allow opt-in raw Bayer capture

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Aug 26, 2026 at 03:22:55PM +0200, Maurizio Casciano wrote:
> AtomISP currently rejects all raw formats and silently substitutes
> YUV420. This prevents userspace camera processing stacks from obtaining
> unprocessed sensor frames.
> 
> Add an allow_raw_output module parameter, disabled by default. When it is
> enabled, enumerate only the raw format matching the sensor media-bus code
> and reconcile raw format requests with the code selected by the sensor.
> 
> Also apply the per-sensor padding discovered by the CSI-2 bridge when
> enumerating and negotiating frame sizes. Existing systems retain the
> global padding and raw-output defaults.
> 
> Tested on a Lenovo Yoga Book YB1-X91L with raw capture from its OV2740
> and OV8858 sensors.

...

>  	u32 min_pad_w = ISP2400_MIN_PAD_W;
>  	u32 min_pad_h = ISP2400_MIN_PAD_H;
>  	struct v4l2_mbus_framefmt *sink;
> +	u32 input_padding_w = input->padding_override ?
> +			      input->padding_w : pad_w;
> +	u32 input_padding_h = input->padding_override ?
> +			      input->padding_h : pad_h;

It's the same condition, split to if-else.

	if (input->padding_override) {
		input_padding_w = input->padding_w;
		input_padding_h = input->padding_h;
	} else {
		input_padding_w = pad_w;
		input_padding_h = pad_h;
	}

>  	if (!input->crop_support) {
> -		*padding_w = pad_w;
> -		*padding_h = pad_h;
> +		*padding_w = input_padding_w;
> +		*padding_h = input_padding_h;
>  		return;
>  	}

Or maybe you can update pad_w and pad_h instead and leave this and might be
other code alone?

...

> +++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h

>  #define ATOMISP_CSS_SUPPORT_YUVPP     1
>  
>  #define ATOMISP_CSS_OUTPUT_SECOND_INDEX     1
> +
> +extern bool atomisp_allow_raw_output;
> +

Location of this doesn't look right. Find a better one.

>  #define ATOMISP_CSS_OUTPUT_DEFAULT_INDEX    0

...

>  struct atomisp_input_subdev {

>  	enum atomisp_camera_port port;
>  	u32 code; /* MEDIA_BUS_FMT_* */

> +	u32 padding_w;
> +	u32 padding_h;

Why not struct v4l2_area?

> +	bool padding_override;
>  	bool binning_support;
>  	bool crop_support;
>  	bool sensor_on;

...

> -		if (format->sh_fmt == IA_CSS_FRAME_FORMAT_RAW)
> -			continue;
> +		if (format->sh_fmt == IA_CSS_FRAME_FORMAT_RAW) {
> +			if (!atomisp_allow_raw_output ||
> +			    format->mbus_code != code.code)
> +				continue;
> +		}

  if (a) { if (b) { ... }} is equivalent to if (a && b) { ... }.

...

> +bool atomisp_allow_raw_output;
> +module_param_named(allow_raw_output, atomisp_allow_raw_output, bool, 0644);
> +MODULE_PARM_DESC(allow_raw_output,
> +		 "allow experimental raw Bayer output (default:false)");

Why do we need this? Can it be enabled a run-time via IOCTL or other means
of ABI?

...

> +	input->padding_override =
> +		atomisp_csi2_get_sensor_padding(input->sensor->dev,
> +						&input->padding_w,
> +						&input->padding_h);

Make a pointer to a struct v4l2_area to be filled by this function.

-- 
With Best Regards,
Andy Shevchenko