[PATCH v2 9/9] media: staging: rkisp1: improve documentation of rkisp1-common.h
Helen Koike
helen.koike at collabora.com
Mon Jul 20 15:36:41 EDT 2020
Hi Dafna,
Thanks for improving this doc!
On 7/18/20 11:59 AM, Dafna Hirschfeld wrote:
> Add more detailed documentation of the structs and functions
> in rkisp1-common.h
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
> ---
> drivers/staging/media/rkisp1/TODO | 1 -
> drivers/staging/media/rkisp1/rkisp1-common.h | 232 +++++++++++++++----
> 2 files changed, 189 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/TODO b/drivers/staging/media/rkisp1/TODO
> index bdb1b8f73556..f0c90d1c86a8 100644
> --- a/drivers/staging/media/rkisp1/TODO
> +++ b/drivers/staging/media/rkisp1/TODO
> @@ -2,7 +2,6 @@
> * Fix checkpatch errors.
> * Review and comment every lock
> * Handle quantization
> -* Document rkisp1-common.h
> * streaming paths (mainpath and selfpath) check if the other path is streaming
> in several places of the code, review this, specially that it doesn't seem it
> supports streaming from both paths at the same time.
> diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
> index 67419cf1a739..2f91630e109c 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-common.h
> +++ b/drivers/staging/media/rkisp1/rkisp1-common.h
> @@ -22,9 +22,14 @@
> #include "rkisp1-regs.h"
> #include "uapi/rkisp1-config.h"
>
> +/*
> + * flags on the 'direction' field in struct 'rkisp1_isp_mbus_info' that indicate
> + * on which pad the media bus format is supported
> + */
> #define RKISP1_ISP_SD_SRC BIT(0)
> #define RKISP1_ISP_SD_SINK BIT(1)
>
> +/* min and max values for the widths and heights of the entities */
> #define RKISP1_ISP_MAX_WIDTH 4032
> #define RKISP1_ISP_MAX_HEIGHT 3024
> #define RKISP1_ISP_MIN_WIDTH 32
> @@ -37,29 +42,36 @@
> #define RKISP1_RSZ_SRC_MIN_WIDTH 32
> #define RKISP1_RSZ_SRC_MIN_HEIGHT 16
>
> +/* the default width and height of all the entities */
> #define RKISP1_DEFAULT_WIDTH 800
> #define RKISP1_DEFAULT_HEIGHT 600
>
> #define RKISP1_DRIVER_NAME "rkisp1"
> #define RKISP1_BUS_INFO "platform:" RKISP1_DRIVER_NAME
>
> +/* maximum number of clocks */
> #define RKISP1_MAX_BUS_CLK 8
>
> +/* a bitmask of the ready stats */
> #define RKISP1_STATS_MEAS_MASK (RKISP1_CIF_ISP_AWB_DONE | \
> RKISP1_CIF_ISP_AFM_FIN | \
> RKISP1_CIF_ISP_EXP_END | \
> RKISP1_CIF_ISP_HIST_MEASURE_RDY)
> +
> +/* enum for the resizer pads */
> enum rkisp1_rsz_pad {
> RKISP1_RSZ_PAD_SINK,
> RKISP1_RSZ_PAD_SRC,
> RKISP1_RSZ_PAD_MAX
> };
>
> +/* enum for the capture id */
> enum rkisp1_stream_id {
> RKISP1_MAINPATH,
> RKISP1_SELFPATH,
> };
>
> +/* bayer patterns */
> enum rkisp1_fmt_raw_pat_type {
> RKISP1_RAW_RGGB = 0,
> RKISP1_RAW_GRBG,
> @@ -67,6 +79,7 @@ enum rkisp1_fmt_raw_pat_type {
> RKISP1_RAW_BGGR,
> };
>
> +/* enum for the isp pads */
> enum rkisp1_isp_pad {
> RKISP1_ISP_PAD_SINK_VIDEO,
> RKISP1_ISP_PAD_SINK_PARAMS,
> @@ -76,8 +89,16 @@ enum rkisp1_isp_pad {
> };
>
> /*
> - * struct rkisp1_sensor_async - Sensor information
> - * @mbus: media bus configuration
> + * struct rkisp1_sensor_async - A container for the v4l2_async_subdev to add to the notifier
> + * of the v4l2-async API
> + *
> + * @asd: async_subdev variable for the sensor
> + * @lanes: number of lanes
> + * @mbus_type: type of bus (currently only CSI2 is supported)
> + * @mbus_flags: media bus (V4L2_MBUS_*) flags
> + * @sd: a pointer to v4l2_subdev struct of the sensor
> + * @pixel_rate_ctrl: pixel rate of the sensor, used to initialize the phy
> + * @dphy: a pointer to the phy
> */
> struct rkisp1_sensor_async {
> struct v4l2_async_subdev asd;
> @@ -90,19 +111,17 @@ struct rkisp1_sensor_async {
> };
>
> /*
> - * struct rkisp1_isp - ISP sub-device
> - *
> - * See Cropping regions of ISP in rkisp1.c for details
> - * @sink_frm: input size, don't have to be equal to sensor size
> - * @sink_fmt: input format
> - * @sink_crop: crop for sink pad
> - * @src_fmt: output format
> - * @src_crop: output size
> - * @ops_lock: ops serialization
> + * struct rkisp1_isp - ISP subdev entity
> *
> - * @is_dphy_errctrl_disabled : if dphy errctrl is disabled (avoid endless interrupt)
> - * @frame_sequence: used to synchronize frame_id between video devices.
> - * @quantization: output quantization
> + * @sd: v4l2_subdev variable
> + * @rkisp1: pointer to rkisp1_device
> + * @pads: media pads
> + * @pad_cfg: pads configurations
> + * @sink_fmt: input format
> + * @src_fmt: output format
> + * @ops_lock: ops serialization
> + * @is_dphy_errctrl_disabled: if dphy errctrl is disabled (avoid endless interrupt)
> + * @frame_sequence: used to synchronize frame_id between video devices.
> */
> struct rkisp1_isp {
> struct v4l2_subdev sd;
> @@ -111,11 +130,19 @@ struct rkisp1_isp {
> struct v4l2_subdev_pad_config pad_cfg[RKISP1_ISP_PAD_MAX];
> const struct rkisp1_isp_mbus_info *sink_fmt;
> const struct rkisp1_isp_mbus_info *src_fmt;
> - struct mutex ops_lock;
> + struct mutex ops_lock; /* serialize the subdevice ops */
> bool is_dphy_errctrl_disabled;
> atomic_t frame_sequence;
> };
>
> +/*
> + * struct rkisp1_vdev_node - Container for the video nodes: params, stats, mainpath, selfpath
> + *
> + * @buf_queue: queue of buffers
> + * @vlock: lock of the video node
> + * @vdev: video node
> + * @pad: media pad
> + */
> struct rkisp1_vdev_node {
> struct vb2_queue buf_queue;
> struct mutex vlock; /* ioctl serialization mutex */
> @@ -123,6 +150,15 @@ struct rkisp1_vdev_node {
> struct media_pad pad;
> };
>
> +/*
> + * struct rkisp1_buffer - A container for the vb2 buffers used by the video devices:
> + * params, stats, mainpath, selfpath
> + *
> + * @vb: vb2 buffer
> + * @queue: entry of the buffer in the queue
> + * @buff_addr: dma addresses of each plane, used only by the capture devices: selfpath, mainpath
> + * @vaddr: virtual address for buffers used by params and stats devices
> + */
> struct rkisp1_buffer {
> struct vb2_v4l2_buffer vb;
> struct list_head queue;
> @@ -132,6 +168,14 @@ struct rkisp1_buffer {
> };
> };
>
> +/*
> + * struct rkisp1_dummy_buffer - A buffer to write the next frame to in case
> + * there are no vb2 buffers available.
> + *
> + * @vaddr: return value of call to dma_alloc_attrs.
> + * @dma_addr: dma address of the buffer.
> + * @size: size of the buffer.
> + */
> struct rkisp1_dummy_buffer {
> void *vaddr;
> dma_addr_t dma_addr;
> @@ -143,17 +187,29 @@ struct rkisp1_device;
> /*
> * struct rkisp1_capture - ISP capture video device
> *
> - * @pix.fmt: buffer format
> - * @pix.info: pixel information
> - * @pix.cfg: pixel configuration
> - *
> - * @buf.lock: lock to protect buf_queue
> - * @buf.queue: queued buffer list
> - * @buf.dummy: dummy space to store dropped data
> + * @vnode: video node
> + * @rkisp1: pointer to rkisp1_device
> + * @id: id of the capture, one of RKISP1_SELFPATH, RKISP1_MAINPATH
> + * @ops: list of callbacks to configure the capture device.
> + * @config: a pointer to the list of registers to configure the capture format.
> + * @is_streaming: device is streaming
> + * @is_stopping: stop_streaming callback was called and the device is in the process of
> + * stopping the streaming.
> + * @done: when stop_streaming callback is called, the device waits for the next irq
> + * handler to stop the streaming by waiting on the 'done' wait queue.
> + * If the irq handler is not called, the stream is stopped by the callback
> + * after timeout.
> + * @sp_y_stride: the selfpath allows to configure a y stride that is longer than the image width.
> + * @buf.lock: lock to protect buf.queue
> + * @buf.queue: queued buffer list
> + * @buf.dummy: dummy space to store dropped data
> *
> - * rkisp1 use shadowsock registers, so it need two buffer at a time
> - * @buf.curr: the buffer used for current frame
> - * @buf.next: the buffer used for next frame
> + * rkisp1 uses shadow registers, so it needs two buffers at a time
> + * @buf.curr: the buffer used for current frame
> + * @buf.next: the buffer used for next frame
> + * @pix.cfg: pixel configuration
> + * @pix.info: a pointer to the v4l2_format_info of the pixel format
> + * @pix.fmt: buffer format
> */
> struct rkisp1_capture {
> struct rkisp1_vdev_node vnode;
> @@ -183,14 +239,18 @@ struct rkisp1_capture {
> /*
> * struct rkisp1_stats - ISP Statistics device
> *
> - * @lock: locks the buffer list 'stat' and 'is_streaming'
> - * @stat: stats buffer list
> + * @vnode: video node
> + * @rkisp1: pointer to the rkisp1 device
> + * @lock: locks the buffer list 'stat' and 'is_streaming'
> + * @stat: queue of rkisp1_buffer
> + * @vdev_fmt: v4l2_format of the metadata format
> + * @is_streaming: device is streaming
> */
> struct rkisp1_stats {
> struct rkisp1_vdev_node vnode;
> struct rkisp1_device *rkisp1;
>
> - spinlock_t lock; /* locks 'is_streaming', and 'stats' */
> + spinlock_t lock; /* locks the buffers list 'stats' and 'is_streaming' */
> struct list_head stat;
> struct v4l2_format vdev_fmt;
> bool is_streaming;
> @@ -199,14 +259,22 @@ struct rkisp1_stats {
> /*
> * struct rkisp1_params - ISP input parameters device
> *
> - * @cur_params: Current ISP parameters
> - * @is_first_params: the first params should take effect immediately
> + * @vnode: video node
> + * @rkisp1: pointer to the rkisp1 device
> + * @config_lock: locks the buffer list 'params' and 'is_streaming'
> + * @params: queue of rkisp1_buffer
> + * @cur_params: the first params values from userspace
> + * @vdev_fmt: v4l2_format of the metadata format
> + * @is_streaming: device is streaming
> + * @is_first_params: the first params should take effect immediately
> + * @quantization: the quantization configured on the isp's src pad
> + * @raw_type: the bayer pattern on the isp video sink pad
> */
> struct rkisp1_params {
> struct rkisp1_vdev_node vnode;
> struct rkisp1_device *rkisp1;
>
> - spinlock_t config_lock;
> + spinlock_t config_lock; /* locks the buffers list 'params' and 'is_streaming' */
> struct list_head params;
> struct rkisp1_params_cfg cur_params;
> struct v4l2_format vdev_fmt;
> @@ -217,6 +285,18 @@ struct rkisp1_params {
> enum rkisp1_fmt_raw_pat_type raw_type;
> };
>
> +/*
> + * struct rkisp1_resizer - Resizer subdev
> + *
> + * @sd: v4l2_subdev variable
> + * @id: id of the resizer, one of RKISP1_SELFPATH, RKISP1_MAINPATH
> + * @rkisp1: pointer to the rkisp1 device
> + * @pads: media pads
> + * @pad_cfg: configurations for the pads
> + * @config: the set of registers to configure the resizer
> + * @pixel_enc: pixel encoding of the resizer
> + * @ops_lock: a lock for the subdev ops
> + */
> struct rkisp1_resizer {
> struct v4l2_subdev sd;
> enum rkisp1_stream_id id;
> @@ -225,9 +305,26 @@ struct rkisp1_resizer {
> struct v4l2_subdev_pad_config pad_cfg[RKISP1_RSZ_PAD_MAX];
> const struct rkisp1_rsz_config *config;
> enum v4l2_pixel_encoding pixel_enc;
> - struct mutex ops_lock;
> + struct mutex ops_lock; /* serialize the subdevice ops */
> };
>
> +/*
> + * struct rkisp1_debug - Values to be exposed on debugfs.
> + * The parameters are counters of the number of times the
> + * event occurred since the driver was loaded.
> + *
> + * @data_loss: loss of data occurred within a line, processing failure
> + * @outform_size_error: size error is generated in outmux submodule
> + * @img_stabilization_size_error: size error is generated in image stabilization submodule
> + * @inform_size_err: size error is generated in inform submodule
> + * @mipi_error: mipi error occurred
> + * @stats_error: writing to the 'Interrupt clear register' did not clear
> + * it in the register 'Masked interrupt status'
> + * @stop_timeout: upon stream stop, the capture waits 1 second for the isr to stop
> + * the stream. This param is incremented in case of timeout.
> + * @frame_drop: a frame was ready but the buffer queue was empty so the frame
> + * was not sent to userspace
> + */
> struct rkisp1_debug {
> struct dentry *debugfs_dir;
> unsigned long data_loss;
> @@ -242,13 +339,24 @@ struct rkisp1_debug {
>
> /*
> * struct rkisp1_device - ISP platform device
> - * @base_addr: base register address
> + *
> + * @base_addr: base register address
> + * @irq: the irq number
> + * @dev: a pointer to the struct device
> + * @clk_size: number of clocks
> + * @clks: array of clocks
> + * @v4l2_dev: v4l2_device variable
> + * @media_dev: media_device variable
> + * @notifier: a notifier to register on the v4l2-async API to be notified on the sensor
> * @active_sensor: sensor in-use, set when streaming on
> - * @isp: ISP sub-device
> - * @rkisp1_capture: capture video device
> - * @stats: ISP statistics output device
> - * @params: ISP input parameters device
> - * @stream_lock: lock to serialize start/stop streaming in capture devices.
> + * @isp: ISP sub-device
> + * @resizer_devs: resizer sub-devices
> + * @capture_devs: capture devices
> + * @stats: ISP statistics metadata capture device
> + * @params: ISP parameters metadata output device
> + * @pipe: media pipeline
> + * @stream_lock: serializes {start/stop}_streaming callbacks between the capture devices.
> + * @debug: debug params to be exposed on debugfs
> */
> struct rkisp1_device {
> void __iomem *base_addr;
> @@ -266,16 +374,20 @@ struct rkisp1_device {
> struct rkisp1_stats stats;
> struct rkisp1_params params;
> struct media_pipeline pipe;
> - struct mutex stream_lock;
> + struct mutex stream_lock; /* serialize {start/stop}_streaming cb between capture devices */
> struct rkisp1_debug debug;
> };
>
> /*
> - * struct rkisp1_isp_mbus_info - ISP pad format info
> + * struct rkisp1_isp_mbus_info - ISP media bus info, Translates media bus code to hardware
> + * format values
> *
> - * Translate mbus_code to hardware format values
> - *
> - * @bus_width: used for parallel
> + * @mbus_code: media bus code
> + * @pixel_enc: pixel encoding
> + * @mipi_dt: mipi data type
yuv_seq missing here.
With this:
Acked-by: Helen Koike <helen.koike at collabora.com>
Thanks!
Helen
> + * @bus_width: bus width
> + * @bayer_pat: bayer pattern
> + * @direction: a bitmask of the flags indicating on which pad the format is supported on
> */
> struct rkisp1_isp_mbus_info {
> u32 mbus_code;
> @@ -298,25 +410,59 @@ static inline u32 rkisp1_read(struct rkisp1_device *rkisp1, unsigned int addr)
> return readl(rkisp1->base_addr + addr);
> }
>
> +/*
> + * rkisp1_sd_adjust_crop_rect - adjust a rectangle to fit into another rectangle.
> + *
> + * @crop: rectangle to adjust.
> + * @bounds: rectangle used as bounds.
> + */
> void rkisp1_sd_adjust_crop_rect(struct v4l2_rect *crop,
> const struct v4l2_rect *bounds);
>
> +/*
> + * rkisp1_sd_adjust_crop - adjust a rectangle to fit into media bus format
> + *
> + * @crop: rectangle to adjust.
> + * @bounds: media bus format used as bounds.
> + */
> void rkisp1_sd_adjust_crop(struct v4l2_rect *crop,
> const struct v4l2_mbus_framefmt *bounds);
>
> +/*
> + * rkisp1_isp_mbus_info - get the isp info of the media bus code
> + *
> + * @mbus_code: the media bus code
> + */
> const struct rkisp1_isp_mbus_info *rkisp1_isp_mbus_info_get(u32 mbus_code);
>
> +/* rkisp1_params_configure - configure the params when stream starts.
> + * This function is called by the isp entity upon stream starts.
> + * The function applies the initial configuration of the parameters.
> + *
> + * @params: pointer to rkisp1_params.
> + * @bayer_pat: the bayer pattern on the isp video sink pad
> + * @quantization: the quantization configured on the isp's src pad
> + */
> void rkisp1_params_configure(struct rkisp1_params *params,
> enum rkisp1_fmt_raw_pat_type bayer_pat,
> enum v4l2_quantization quantization);
> +
> +/* rkisp1_params_disable - disable all parameters.
> + * This function is called by the isp entity upon stream start
> + * when capturing bayer format.
> + *
> + * @params: pointer to rkisp1_params.
> + */
> void rkisp1_params_disable(struct rkisp1_params *params);
>
> +/* irq handlers */
> void rkisp1_isp_isr(struct rkisp1_device *rkisp1);
> void rkisp1_mipi_isr(struct rkisp1_device *rkisp1);
> void rkisp1_capture_isr(struct rkisp1_device *rkisp1);
> void rkisp1_stats_isr(struct rkisp1_stats *stats, u32 isp_ris);
> void rkisp1_params_isr(struct rkisp1_device *rkisp1, u32 isp_mis);
>
> +/* register/unregisters functions of the entities */
> int rkisp1_capture_devs_register(struct rkisp1_device *rkisp1);
> void rkisp1_capture_devs_unregister(struct rkisp1_device *rkisp1);
>
>
More information about the Linux-rockchip
mailing list