* [PATCH v2 1/2] [media] tc358743: put lanes in STOP state before starting streaming @ 2017-02-13 9:24 Philipp Zabel 2017-02-13 9:24 ` [PATCH v2 2/2] [media] tc358743: extend colorimetry support Philipp Zabel 0 siblings, 1 reply; 4+ messages in thread From: Philipp Zabel @ 2017-02-13 9:24 UTC (permalink / raw) To: linux-media; +Cc: Hans Verkuil, Mats Randgaard, Philipp Zabel Without calling tc358743_set_csi after stopping streaming (or calling tc358743_s_dv_timings or tc358743_set_fmt from userspace after stopping the stream), the i.MX6 MIPI CSI2 input fails waiting for lanes to enter STOP state when streaming is started again. Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- drivers/media/i2c/tc358743.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index f569a05fe1054..64a97bbbd00a8 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -1459,6 +1459,10 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd, static int tc358743_s_stream(struct v4l2_subdev *sd, int enable) { enable_stream(sd, enable); + if (!enable) { + /* Put all lanes in PL-11 state (STOPSTATE) */ + tc358743_set_csi(sd); + } return 0; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] [media] tc358743: extend colorimetry support 2017-02-13 9:24 [PATCH v2 1/2] [media] tc358743: put lanes in STOP state before starting streaming Philipp Zabel @ 2017-02-13 9:24 ` Philipp Zabel 2017-02-13 10:05 ` Hans Verkuil 0 siblings, 1 reply; 4+ messages in thread From: Philipp Zabel @ 2017-02-13 9:24 UTC (permalink / raw) To: linux-media; +Cc: Hans Verkuil, Mats Randgaard, Philipp Zabel The video output format can freely be chosen to be 24-bit SRGB or 16-bit YUV 4:2:2 in either SMPTE170M or REC709 color space. In all three modes the output can be full or limited range. Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- Changes since v1: - Determine colorspace and xfer_func from the S_V_COLOR (HDMI input colorspace) bitfield. This defaults to SRGB full range if no input is connected, or if the input does not send AVI infoframes (DVI). - Control ycbcr_enc and quantization via VOUT_COLOR_SEL. This can be set via set_fmt, with some limitations: - Disallow YUV to RGB conversion, which is apparently not supported by the hardware. - Limit V4L2_YCBCR_ENC_601/709 encodings to limited quantization range and limit V4L2_YCBCR_ENC_XV601/XV709 encodings to full quantization range - Default to limited range for SRGB and Adobe RGB if quantization is not set explicitly. --- drivers/media/i2c/tc358743.c | 148 ++++++++++++++++++++++++++++++++------ drivers/media/i2c/tc358743_regs.h | 8 +++ 2 files changed, 135 insertions(+), 21 deletions(-) diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index 64a97bbbd00a8..9f191735be805 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -96,6 +96,7 @@ struct tc358743_state { struct v4l2_dv_timings timings; u32 mbus_fmt_code; + u8 vout_color_sel; u8 csi_lanes_in_use; struct gpio_desc *reset_gpio; @@ -628,7 +629,7 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd) ~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff, MASK_SEL422 | MASK_VOUT_422FIL_100); i2c_wr8_and_or(sd, VI_REP, ~MASK_VOUT_COLOR_SEL & 0xff, - MASK_VOUT_COLOR_601_YCBCR_LIMITED); + state->vout_color_sel); mutex_lock(&state->confctl_mutex); i2c_wr16_and_or(sd, CONFCTL, ~MASK_YCBCRFMT, MASK_YCBCRFMT_422_8_BIT); @@ -640,7 +641,7 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd) ~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff, 0x00); i2c_wr8_and_or(sd, VI_REP, ~MASK_VOUT_COLOR_SEL & 0xff, - MASK_VOUT_COLOR_RGB_FULL); + state->vout_color_sel); mutex_lock(&state->confctl_mutex); i2c_wr16_and_or(sd, CONFCTL, ~MASK_YCBCRFMT, 0); mutex_unlock(&state->confctl_mutex); @@ -1096,11 +1097,17 @@ static int tc358743_log_status(struct v4l2_subdev *sd) uint8_t hdmi_sys_status = i2c_rd8(sd, SYS_STATUS); uint16_t sysctl = i2c_rd16(sd, SYSCTL); u8 vi_status3 = i2c_rd8(sd, VI_STATUS3); + u8 vi_rep = i2c_rd8(sd, VI_REP); const int deep_color_mode[4] = { 8, 10, 12, 16 }; static const char * const input_color_space[] = { "RGB", "YCbCr 601", "Adobe RGB", "YCbCr 709", "NA (4)", "xvYCC 601", "NA(6)", "xvYCC 709", "NA(8)", "sYCC601", "NA(10)", "NA(11)", "NA(12)", "Adobe YCC 601"}; + static const char * const output_color_space[8] = { + "full range (0-255)", "limited range (16-235)", + "Bt.601 (0-255)", "Bt.601 (16-235)", + "Bt.709 (0-255)", "Bt.709 (16-235)", + "full to limited range", "limited to full range"}; v4l2_info(sd, "-----Chip status-----\n"); v4l2_info(sd, "Chip ID: 0x%02x\n", @@ -1159,11 +1166,12 @@ static int tc358743_log_status(struct v4l2_subdev *sd) v4l2_info(sd, "Stopped: %s\n", (i2c_rd16(sd, CSI_STATUS) & MASK_S_HLT) ? "yes" : "no"); - v4l2_info(sd, "Color space: %s\n", + v4l2_info(sd, "Color space: %s %s\n", state->mbus_fmt_code == MEDIA_BUS_FMT_UYVY8_1X16 ? "YCbCr 422 16-bit" : state->mbus_fmt_code == MEDIA_BUS_FMT_RGB888_1X24 ? - "RGB 888 24-bit" : "Unsupported"); + "RGB 888 24-bit" : "Unsupported", + output_color_space[(vi_rep & MASK_VOUT_COLOR_SEL) >> 5]); v4l2_info(sd, "-----%s status-----\n", is_hdmi(sd) ? "HDMI" : "DVI-D"); v4l2_info(sd, "HDCP encrypted content: %s\n", @@ -1469,38 +1477,88 @@ static int tc358743_s_stream(struct v4l2_subdev *sd, int enable) /* --------------- PAD OPS --------------- */ -static int tc358743_get_fmt(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, - struct v4l2_subdev_format *format) +static void tc358743_get_csi_color_space(struct v4l2_subdev *sd, + struct v4l2_mbus_framefmt *format) { - struct tc358743_state *state = to_state(sd); + u8 vi_status3 = i2c_rd8(sd, VI_STATUS3); u8 vi_rep = i2c_rd8(sd, VI_REP); - if (format->pad != 0) - return -EINVAL; + switch (vi_status3 & MASK_S_V_COLOR) { + default: + case MASK_S_V_COLOR_RGB: + case MASK_S_V_COLOR_SYCC601: + format->colorspace = V4L2_COLORSPACE_SRGB; + format->xfer_func = V4L2_XFER_FUNC_SRGB; + break; + case MASK_S_V_COLOR_YCBCR601: + case MASK_S_V_COLOR_XVYCC601: + format->colorspace = V4L2_COLORSPACE_SMPTE170M; + format->xfer_func = V4L2_XFER_FUNC_709; + break; + case MASK_S_V_COLOR_YCBCR709: + case MASK_S_V_COLOR_XVYCC709: + format->colorspace = V4L2_COLORSPACE_REC709; + format->xfer_func = V4L2_XFER_FUNC_709; + break; + case MASK_S_V_COLOR_ADOBERGB: + case MASK_S_V_COLOR_ADOBEYCC601: + format->colorspace = V4L2_COLORSPACE_ADOBERGB; + format->xfer_func = V4L2_XFER_FUNC_ADOBERGB; + break; + } - format->format.code = state->mbus_fmt_code; - format->format.width = state->timings.bt.width; - format->format.height = state->timings.bt.height; - format->format.field = V4L2_FIELD_NONE; + format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(format->colorspace); switch (vi_rep & MASK_VOUT_COLOR_SEL) { + default: case MASK_VOUT_COLOR_RGB_FULL: + format->ycbcr_enc = V4L2_YCBCR_ENC_601; + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; + break; case MASK_VOUT_COLOR_RGB_LIMITED: - format->format.colorspace = V4L2_COLORSPACE_SRGB; + format->ycbcr_enc = V4L2_YCBCR_ENC_601; + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; break; case MASK_VOUT_COLOR_601_YCBCR_LIMITED: + format->ycbcr_enc = V4L2_YCBCR_ENC_601; + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; + break; case MASK_VOUT_COLOR_601_YCBCR_FULL: - format->format.colorspace = V4L2_COLORSPACE_SMPTE170M; + format->ycbcr_enc = V4L2_YCBCR_ENC_XV601; + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; break; case MASK_VOUT_COLOR_709_YCBCR_FULL: + format->ycbcr_enc = V4L2_YCBCR_ENC_XV709; + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; + break; case MASK_VOUT_COLOR_709_YCBCR_LIMITED: - format->format.colorspace = V4L2_COLORSPACE_REC709; + format->ycbcr_enc = V4L2_YCBCR_ENC_709; + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; break; - default: - format->format.colorspace = 0; + case MASK_VOUT_COLOR_FULL_TO_LIMITED: + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; + break; + case MASK_VOUT_COLOR_LIMITED_TO_FULL: + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; break; } +} + +static int tc358743_get_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *format) +{ + struct tc358743_state *state = to_state(sd); + + if (format->pad != 0) + return -EINVAL; + + format->format.code = state->mbus_fmt_code; + format->format.width = state->timings.bt.width; + format->format.height = state->timings.bt.height; + format->format.field = V4L2_FIELD_NONE; + + tc358743_get_csi_color_space(sd, &format->format); return 0; } @@ -1512,25 +1570,72 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd, struct tc358743_state *state = to_state(sd); u32 code = format->format.code; /* is overwritten by get_fmt */ + enum v4l2_ycbcr_encoding ycbcr_enc = format->format.ycbcr_enc; + enum v4l2_quantization quantization = format->format.quantization; int ret = tc358743_get_fmt(sd, cfg, format); - - format->format.code = code; + enum v4l2_colorspace colorspace; + u8 vout_color_sel; if (ret) return ret; + colorspace = format->format.colorspace; + switch (code) { case MEDIA_BUS_FMT_RGB888_1X24: + if (colorspace == V4L2_COLORSPACE_SRGB || + colorspace == V4L2_COLORSPACE_ADOBERGB) { + ycbcr_enc = V4L2_YCBCR_ENC_601; + if (quantization == V4L2_QUANTIZATION_FULL_RANGE) { + vout_color_sel = MASK_VOUT_COLOR_RGB_FULL; + } else { + quantization = V4L2_QUANTIZATION_LIM_RANGE; + vout_color_sel = MASK_VOUT_COLOR_RGB_LIMITED; + } + break; + } + /* + * Since color space conversion to RGB is not supported, + * fall through for YUV input. + */ + code = MEDIA_BUS_FMT_UYVY8_1X16; case MEDIA_BUS_FMT_UYVY8_1X16: + if (ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) + ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(colorspace); + switch (ycbcr_enc) { + case V4L2_YCBCR_ENC_601: + quantization = V4L2_QUANTIZATION_LIM_RANGE; + vout_color_sel = MASK_VOUT_COLOR_601_YCBCR_LIMITED; + break; + case V4L2_YCBCR_ENC_709: + quantization = V4L2_QUANTIZATION_LIM_RANGE; + vout_color_sel = MASK_VOUT_COLOR_709_YCBCR_LIMITED; + break; + case V4L2_YCBCR_ENC_XV601: + quantization = V4L2_QUANTIZATION_FULL_RANGE; + vout_color_sel = MASK_VOUT_COLOR_601_YCBCR_FULL; + break; + case V4L2_YCBCR_ENC_XV709: + quantization = V4L2_QUANTIZATION_FULL_RANGE; + vout_color_sel = MASK_VOUT_COLOR_709_YCBCR_FULL; + break; + default: + return -EINVAL; + } break; default: return -EINVAL; } + format->format.code = code; + format->format.ycbcr_enc = ycbcr_enc; + format->format.quantization = quantization; + if (format->which == V4L2_SUBDEV_FORMAT_TRY) return 0; state->mbus_fmt_code = format->format.code; + state->vout_color_sel = vout_color_sel; enable_stream(sd, false); tc358743_set_pll(sd); @@ -1898,6 +2003,7 @@ static int tc358743_probe(struct i2c_client *client, tc358743_s_dv_timings(sd, &default_timing); state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24; + state->vout_color_sel = MASK_VOUT_COLOR_RGB_FULL; tc358743_set_csi_color_space(sd); tc358743_init_interrupts(sd); diff --git a/drivers/media/i2c/tc358743_regs.h b/drivers/media/i2c/tc358743_regs.h index 657ef50f215f5..8dae3d34c6270 100644 --- a/drivers/media/i2c/tc358743_regs.h +++ b/drivers/media/i2c/tc358743_regs.h @@ -337,6 +337,14 @@ #define VI_STATUS3 0x8528 #define MASK_S_V_COLOR 0x1e +#define MASK_S_V_COLOR_RGB 0x00 +#define MASK_S_V_COLOR_YCBCR601 0x02 +#define MASK_S_V_COLOR_YCBCR709 0x06 +#define MASK_S_V_COLOR_ADOBERGB 0x04 +#define MASK_S_V_COLOR_XVYCC601 0x0a +#define MASK_S_V_COLOR_XVYCC709 0x0e +#define MASK_S_V_COLOR_SYCC601 0x12 +#define MASK_S_V_COLOR_ADOBEYCC601 0x1a #define MASK_LIMITED 0x01 #define PHY_CTL0 0x8531 -- 2.11.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] [media] tc358743: extend colorimetry support 2017-02-13 9:24 ` [PATCH v2 2/2] [media] tc358743: extend colorimetry support Philipp Zabel @ 2017-02-13 10:05 ` Hans Verkuil 2017-02-13 10:39 ` Philipp Zabel 0 siblings, 1 reply; 4+ messages in thread From: Hans Verkuil @ 2017-02-13 10:05 UTC (permalink / raw) To: Philipp Zabel, linux-media; +Cc: Mats Randgaard On 02/13/2017 10:24 AM, Philipp Zabel wrote: > The video output format can freely be chosen to be 24-bit SRGB or 16-bit > YUV 4:2:2 in either SMPTE170M or REC709 color space. In all three modes > the output can be full or limited range. > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > Changes since v1: > - Determine colorspace and xfer_func from the S_V_COLOR (HDMI input > colorspace) bitfield. This defaults to SRGB full range if no input > is connected, or if the input does not send AVI infoframes (DVI). > - Control ycbcr_enc and quantization via VOUT_COLOR_SEL. This can be > set via set_fmt, with some limitations: > - Disallow YUV to RGB conversion, which is apparently not supported by the > hardware. > - Limit V4L2_YCBCR_ENC_601/709 encodings to limited quantization range and > limit V4L2_YCBCR_ENC_XV601/XV709 encodings to full quantization range > - Default to limited range for SRGB and Adobe RGB if quantization is not > set explicitly. > --- > drivers/media/i2c/tc358743.c | 148 ++++++++++++++++++++++++++++++++------ > drivers/media/i2c/tc358743_regs.h | 8 +++ > 2 files changed, 135 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c > index 64a97bbbd00a8..9f191735be805 100644 > --- a/drivers/media/i2c/tc358743.c > +++ b/drivers/media/i2c/tc358743.c > @@ -96,6 +96,7 @@ struct tc358743_state { > > struct v4l2_dv_timings timings; > u32 mbus_fmt_code; > + u8 vout_color_sel; > u8 csi_lanes_in_use; > > struct gpio_desc *reset_gpio; > @@ -628,7 +629,7 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd) > ~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff, > MASK_SEL422 | MASK_VOUT_422FIL_100); > i2c_wr8_and_or(sd, VI_REP, ~MASK_VOUT_COLOR_SEL & 0xff, > - MASK_VOUT_COLOR_601_YCBCR_LIMITED); > + state->vout_color_sel); > mutex_lock(&state->confctl_mutex); > i2c_wr16_and_or(sd, CONFCTL, ~MASK_YCBCRFMT, > MASK_YCBCRFMT_422_8_BIT); > @@ -640,7 +641,7 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd) > ~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff, > 0x00); > i2c_wr8_and_or(sd, VI_REP, ~MASK_VOUT_COLOR_SEL & 0xff, > - MASK_VOUT_COLOR_RGB_FULL); > + state->vout_color_sel); > mutex_lock(&state->confctl_mutex); > i2c_wr16_and_or(sd, CONFCTL, ~MASK_YCBCRFMT, 0); > mutex_unlock(&state->confctl_mutex); > @@ -1096,11 +1097,17 @@ static int tc358743_log_status(struct v4l2_subdev *sd) > uint8_t hdmi_sys_status = i2c_rd8(sd, SYS_STATUS); > uint16_t sysctl = i2c_rd16(sd, SYSCTL); > u8 vi_status3 = i2c_rd8(sd, VI_STATUS3); > + u8 vi_rep = i2c_rd8(sd, VI_REP); > const int deep_color_mode[4] = { 8, 10, 12, 16 }; > static const char * const input_color_space[] = { > "RGB", "YCbCr 601", "Adobe RGB", "YCbCr 709", "NA (4)", > "xvYCC 601", "NA(6)", "xvYCC 709", "NA(8)", "sYCC601", > "NA(10)", "NA(11)", "NA(12)", "Adobe YCC 601"}; > + static const char * const output_color_space[8] = { > + "full range (0-255)", "limited range (16-235)", > + "Bt.601 (0-255)", "Bt.601 (16-235)", > + "Bt.709 (0-255)", "Bt.709 (16-235)", > + "full to limited range", "limited to full range"}; > > v4l2_info(sd, "-----Chip status-----\n"); > v4l2_info(sd, "Chip ID: 0x%02x\n", > @@ -1159,11 +1166,12 @@ static int tc358743_log_status(struct v4l2_subdev *sd) > v4l2_info(sd, "Stopped: %s\n", > (i2c_rd16(sd, CSI_STATUS) & MASK_S_HLT) ? > "yes" : "no"); > - v4l2_info(sd, "Color space: %s\n", > + v4l2_info(sd, "Color space: %s %s\n", > state->mbus_fmt_code == MEDIA_BUS_FMT_UYVY8_1X16 ? > "YCbCr 422 16-bit" : > state->mbus_fmt_code == MEDIA_BUS_FMT_RGB888_1X24 ? > - "RGB 888 24-bit" : "Unsupported"); > + "RGB 888 24-bit" : "Unsupported", > + output_color_space[(vi_rep & MASK_VOUT_COLOR_SEL) >> 5]); > > v4l2_info(sd, "-----%s status-----\n", is_hdmi(sd) ? "HDMI" : "DVI-D"); > v4l2_info(sd, "HDCP encrypted content: %s\n", > @@ -1469,38 +1477,88 @@ static int tc358743_s_stream(struct v4l2_subdev *sd, int enable) > > /* --------------- PAD OPS --------------- */ > > -static int tc358743_get_fmt(struct v4l2_subdev *sd, > - struct v4l2_subdev_pad_config *cfg, > - struct v4l2_subdev_format *format) > +static void tc358743_get_csi_color_space(struct v4l2_subdev *sd, > + struct v4l2_mbus_framefmt *format) > { > - struct tc358743_state *state = to_state(sd); > + u8 vi_status3 = i2c_rd8(sd, VI_STATUS3); > u8 vi_rep = i2c_rd8(sd, VI_REP); > > - if (format->pad != 0) > - return -EINVAL; > + switch (vi_status3 & MASK_S_V_COLOR) { > + default: > + case MASK_S_V_COLOR_RGB: > + case MASK_S_V_COLOR_SYCC601: > + format->colorspace = V4L2_COLORSPACE_SRGB; > + format->xfer_func = V4L2_XFER_FUNC_SRGB; > + break; > + case MASK_S_V_COLOR_YCBCR601: > + case MASK_S_V_COLOR_XVYCC601: > + format->colorspace = V4L2_COLORSPACE_SMPTE170M; Not correct. XVYCC601 uses the REC709 colorspace. The XVYCC formats are only defined for the REC709 colorspace and only differ in the YCbCr encoding that they use. > + format->xfer_func = V4L2_XFER_FUNC_709; > + break; > + case MASK_S_V_COLOR_YCBCR709: > + case MASK_S_V_COLOR_XVYCC709: > + format->colorspace = V4L2_COLORSPACE_REC709; > + format->xfer_func = V4L2_XFER_FUNC_709; > + break; > + case MASK_S_V_COLOR_ADOBERGB: > + case MASK_S_V_COLOR_ADOBEYCC601: > + format->colorspace = V4L2_COLORSPACE_ADOBERGB; > + format->xfer_func = V4L2_XFER_FUNC_ADOBERGB; > + break; > + } > > - format->format.code = state->mbus_fmt_code; > - format->format.width = state->timings.bt.width; > - format->format.height = state->timings.bt.height; > - format->format.field = V4L2_FIELD_NONE; > + format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(format->colorspace); > > switch (vi_rep & MASK_VOUT_COLOR_SEL) { > + default: > case MASK_VOUT_COLOR_RGB_FULL: > + format->ycbcr_enc = V4L2_YCBCR_ENC_601; > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > + break; > case MASK_VOUT_COLOR_RGB_LIMITED: > - format->format.colorspace = V4L2_COLORSPACE_SRGB; > + format->ycbcr_enc = V4L2_YCBCR_ENC_601; > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > break; > case MASK_VOUT_COLOR_601_YCBCR_LIMITED: > + format->ycbcr_enc = V4L2_YCBCR_ENC_601; > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > + break; > case MASK_VOUT_COLOR_601_YCBCR_FULL: > - format->format.colorspace = V4L2_COLORSPACE_SMPTE170M; > + format->ycbcr_enc = V4L2_YCBCR_ENC_XV601; This isn't right. Only use V4L2_YCBCR_ENC_XV601 (or XV709) if this is signaled in the InfoFrame. Full Range V4L2_YCBCR_ENC_601 != Full Range V4L2_YCBCR_ENC_XV601. XV601 is similar to Limited Range 601, except that it also utilizes values 0-15 and 241-255, thus allowing for a wider gamut of colors. > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > break; > case MASK_VOUT_COLOR_709_YCBCR_FULL: > + format->ycbcr_enc = V4L2_YCBCR_ENC_XV709; > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > + break; > case MASK_VOUT_COLOR_709_YCBCR_LIMITED: > - format->format.colorspace = V4L2_COLORSPACE_REC709; > + format->ycbcr_enc = V4L2_YCBCR_ENC_709; > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > break; > - default: > - format->format.colorspace = 0; > + case MASK_VOUT_COLOR_FULL_TO_LIMITED: > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > + break; > + case MASK_VOUT_COLOR_LIMITED_TO_FULL: > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > break; > } > +} > + > +static int tc358743_get_fmt(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *format) > +{ > + struct tc358743_state *state = to_state(sd); > + > + if (format->pad != 0) > + return -EINVAL; > + > + format->format.code = state->mbus_fmt_code; > + format->format.width = state->timings.bt.width; > + format->format.height = state->timings.bt.height; > + format->format.field = V4L2_FIELD_NONE; > + > + tc358743_get_csi_color_space(sd, &format->format); > > return 0; > } > @@ -1512,25 +1570,72 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd, > struct tc358743_state *state = to_state(sd); > > u32 code = format->format.code; /* is overwritten by get_fmt */ > + enum v4l2_ycbcr_encoding ycbcr_enc = format->format.ycbcr_enc; > + enum v4l2_quantization quantization = format->format.quantization; > int ret = tc358743_get_fmt(sd, cfg, format); > - > - format->format.code = code; > + enum v4l2_colorspace colorspace; > + u8 vout_color_sel; > > if (ret) > return ret; > > + colorspace = format->format.colorspace; > + > switch (code) { > case MEDIA_BUS_FMT_RGB888_1X24: > + if (colorspace == V4L2_COLORSPACE_SRGB || > + colorspace == V4L2_COLORSPACE_ADOBERGB) { > + ycbcr_enc = V4L2_YCBCR_ENC_601; > + if (quantization == V4L2_QUANTIZATION_FULL_RANGE) { > + vout_color_sel = MASK_VOUT_COLOR_RGB_FULL; > + } else { > + quantization = V4L2_QUANTIZATION_LIM_RANGE; > + vout_color_sel = MASK_VOUT_COLOR_RGB_LIMITED; > + } > + break; > + } > + /* > + * Since color space conversion to RGB is not supported, > + * fall through for YUV input. > + */ > + code = MEDIA_BUS_FMT_UYVY8_1X16; > case MEDIA_BUS_FMT_UYVY8_1X16: > + if (ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) > + ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(colorspace); > + switch (ycbcr_enc) { > + case V4L2_YCBCR_ENC_601: > + quantization = V4L2_QUANTIZATION_LIM_RANGE; > + vout_color_sel = MASK_VOUT_COLOR_601_YCBCR_LIMITED; > + break; > + case V4L2_YCBCR_ENC_709: > + quantization = V4L2_QUANTIZATION_LIM_RANGE; > + vout_color_sel = MASK_VOUT_COLOR_709_YCBCR_LIMITED; > + break; > + case V4L2_YCBCR_ENC_XV601: > + quantization = V4L2_QUANTIZATION_FULL_RANGE; > + vout_color_sel = MASK_VOUT_COLOR_601_YCBCR_FULL; > + break; > + case V4L2_YCBCR_ENC_XV709: > + quantization = V4L2_QUANTIZATION_FULL_RANGE; > + vout_color_sel = MASK_VOUT_COLOR_709_YCBCR_FULL; > + break; > + default: > + return -EINVAL; > + } > break; > default: > return -EINVAL; > } > > + format->format.code = code; > + format->format.ycbcr_enc = ycbcr_enc; > + format->format.quantization = quantization; > + > if (format->which == V4L2_SUBDEV_FORMAT_TRY) > return 0; > > state->mbus_fmt_code = format->format.code; > + state->vout_color_sel = vout_color_sel; > > enable_stream(sd, false); > tc358743_set_pll(sd); > @@ -1898,6 +2003,7 @@ static int tc358743_probe(struct i2c_client *client, > tc358743_s_dv_timings(sd, &default_timing); > > state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24; > + state->vout_color_sel = MASK_VOUT_COLOR_RGB_FULL; > tc358743_set_csi_color_space(sd); > > tc358743_init_interrupts(sd); > diff --git a/drivers/media/i2c/tc358743_regs.h b/drivers/media/i2c/tc358743_regs.h > index 657ef50f215f5..8dae3d34c6270 100644 > --- a/drivers/media/i2c/tc358743_regs.h > +++ b/drivers/media/i2c/tc358743_regs.h > @@ -337,6 +337,14 @@ > > #define VI_STATUS3 0x8528 > #define MASK_S_V_COLOR 0x1e > +#define MASK_S_V_COLOR_RGB 0x00 > +#define MASK_S_V_COLOR_YCBCR601 0x02 > +#define MASK_S_V_COLOR_YCBCR709 0x06 > +#define MASK_S_V_COLOR_ADOBERGB 0x04 > +#define MASK_S_V_COLOR_XVYCC601 0x0a > +#define MASK_S_V_COLOR_XVYCC709 0x0e > +#define MASK_S_V_COLOR_SYCC601 0x12 > +#define MASK_S_V_COLOR_ADOBEYCC601 0x1a > #define MASK_LIMITED 0x01 > > #define PHY_CTL0 0x8531 > Except for the noted issues this looks good to me. Regards, Hans ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] [media] tc358743: extend colorimetry support 2017-02-13 10:05 ` Hans Verkuil @ 2017-02-13 10:39 ` Philipp Zabel 0 siblings, 0 replies; 4+ messages in thread From: Philipp Zabel @ 2017-02-13 10:39 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media, Mats Randgaard On Mon, 2017-02-13 at 11:05 +0100, Hans Verkuil wrote: [...] > > @@ -1469,38 +1477,88 @@ static int tc358743_s_stream(struct v4l2_subdev *sd, int enable) > > > > /* --------------- PAD OPS --------------- */ > > > > -static int tc358743_get_fmt(struct v4l2_subdev *sd, > > - struct v4l2_subdev_pad_config *cfg, > > - struct v4l2_subdev_format *format) > > +static void tc358743_get_csi_color_space(struct v4l2_subdev *sd, > > + struct v4l2_mbus_framefmt *format) > > { > > - struct tc358743_state *state = to_state(sd); > > + u8 vi_status3 = i2c_rd8(sd, VI_STATUS3); > > u8 vi_rep = i2c_rd8(sd, VI_REP); > > > > - if (format->pad != 0) > > - return -EINVAL; > > + switch (vi_status3 & MASK_S_V_COLOR) { > > + default: > > + case MASK_S_V_COLOR_RGB: > > + case MASK_S_V_COLOR_SYCC601: > > + format->colorspace = V4L2_COLORSPACE_SRGB; > > + format->xfer_func = V4L2_XFER_FUNC_SRGB; > > + break; > > + case MASK_S_V_COLOR_YCBCR601: > > + case MASK_S_V_COLOR_XVYCC601: > > + format->colorspace = V4L2_COLORSPACE_SMPTE170M; > > Not correct. XVYCC601 uses the REC709 colorspace. The XVYCC formats > are only defined for the REC709 colorspace and only differ in the > YCbCr encoding that they use. I'll move the XVYCC601 case down to join YCBCR709 and XVYCC709. > > + format->xfer_func = V4L2_XFER_FUNC_709; > > + break; > > + case MASK_S_V_COLOR_YCBCR709: > > + case MASK_S_V_COLOR_XVYCC709: > > + format->colorspace = V4L2_COLORSPACE_REC709; > > + format->xfer_func = V4L2_XFER_FUNC_709; > > + break; > > + case MASK_S_V_COLOR_ADOBERGB: > > + case MASK_S_V_COLOR_ADOBEYCC601: > > + format->colorspace = V4L2_COLORSPACE_ADOBERGB; > > + format->xfer_func = V4L2_XFER_FUNC_ADOBERGB; > > + break; > > + } > > > > - format->format.code = state->mbus_fmt_code; > > - format->format.width = state->timings.bt.width; > > - format->format.height = state->timings.bt.height; > > - format->format.field = V4L2_FIELD_NONE; > > + format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(format->colorspace); > > > > switch (vi_rep & MASK_VOUT_COLOR_SEL) { > > + default: > > case MASK_VOUT_COLOR_RGB_FULL: > > + format->ycbcr_enc = V4L2_YCBCR_ENC_601; > > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > > + break; > > case MASK_VOUT_COLOR_RGB_LIMITED: > > - format->format.colorspace = V4L2_COLORSPACE_SRGB; > > + format->ycbcr_enc = V4L2_YCBCR_ENC_601; > > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > > break; > > case MASK_VOUT_COLOR_601_YCBCR_LIMITED: > > + format->ycbcr_enc = V4L2_YCBCR_ENC_601; > > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > > + break; > > case MASK_VOUT_COLOR_601_YCBCR_FULL: > > - format->format.colorspace = V4L2_COLORSPACE_SMPTE170M; > > + format->ycbcr_enc = V4L2_YCBCR_ENC_XV601; > > This isn't right. Only use V4L2_YCBCR_ENC_XV601 (or XV709) if this is > signaled in the InfoFrame. > > Full Range V4L2_YCBCR_ENC_601 != Full Range V4L2_YCBCR_ENC_XV601. > > XV601 is similar to Limited Range 601, except that it also utilizes > values 0-15 and 241-255, thus allowing for a wider gamut of colors. Thanks. With that explanation, I think I should just change this to V4L2_YCBCR_ENC_601 and limit the VOUT_COLOR_RGB/601/709 selections to RGB input. > > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > > break; > > case MASK_VOUT_COLOR_709_YCBCR_FULL: > > + format->ycbcr_enc = V4L2_YCBCR_ENC_XV709; > > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > > + break; > > case MASK_VOUT_COLOR_709_YCBCR_LIMITED: > > - format->format.colorspace = V4L2_COLORSPACE_REC709; > > + format->ycbcr_enc = V4L2_YCBCR_ENC_709; > > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > > break; > > - default: > > - format->format.colorspace = 0; > > + case MASK_VOUT_COLOR_FULL_TO_LIMITED: > > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > > + break; > > + case MASK_VOUT_COLOR_LIMITED_TO_FULL: > > + format->quantization = V4L2_QUANTIZATION_FULL_RANGE; > > break; Then I'll use FULL_TO_LIMITED / LIMITED_TO_FULL to for YUV inputs and correctly set ycbcr_enc and quantization depending on this and input colorspace. [...] > Except for the noted issues this looks good to me. Thank you for the review. regards Philipp ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-13 10:39 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-13 9:24 [PATCH v2 1/2] [media] tc358743: put lanes in STOP state before starting streaming Philipp Zabel 2017-02-13 9:24 ` [PATCH v2 2/2] [media] tc358743: extend colorimetry support Philipp Zabel 2017-02-13 10:05 ` Hans Verkuil 2017-02-13 10:39 ` Philipp Zabel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.