media: hws: report measured colorimetry

[PATCH] media: hws: report measured colorimetry

Posted by Ben Hoff 1 month, 3 weeks ago

At 1920x1080, the driver advertises V4L2_COLORSPACE_REC709 with
V4L2_YCBCR_ENC_DEFAULT. The default maps to a Rec.709 matrix, but
controlled A/B captures using the in-tree and baseline drivers show that
both produce YUYV samples using a BT.601 matrix and full-range
quantization. The captures differ by at most one code value, whereas a
601-to-709 conversion would move primary luma values by roughly 11 to 33
codes.

Set the Y'CbCr encoding explicitly to V4L2_YCBCR_ENC_601 at every
resolution. Keep the resolution-based colorspace so HD retains presumed
Rec.709 primaries and transfer characteristics while SD continues to use
SMPTE 170M. Refresh the colorspace on detected geometry changes so an
SD/HD transition does not leave stale metadata.

The decoder mode register is undocumented. Stop claiming that value 0x13
selects BT.709; both drivers use that value while producing the measured
output.

Fixes: ba07fd2f5742 ("media: pci: add AVMatrix HWS capture driver")
Assisted-by: Codex:gpt-5.6
Signed-off-by: Ben Hoff <hoff.benjamin.k@gmail.com>
---
 drivers/media/pci/hws/hws.h            | 4 ++--
 drivers/media/pci/hws/hws_reg.h        | 3 ++-
 drivers/media/pci/hws/hws_v4l2_ioctl.c | 5 +++--
 drivers/media/pci/hws/hws_video.c      | 4 +++-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/pci/hws/hws.h b/drivers/media/pci/hws/hws.h
index 8fbe1fe27844..d87d52674b69 100644
--- a/drivers/media/pci/hws/hws.h
+++ b/drivers/media/pci/hws/hws.h
@@ -33,8 +33,8 @@ struct hws_pix_state {
 	u32 sizeimage;		/* full frame */
 	enum v4l2_field field;	/* V4L2_FIELD_NONE or INTERLACED */
 	enum v4l2_colorspace colorspace;	/* e.g., REC709 */
-	enum v4l2_ycbcr_encoding ycbcr_enc;	/* V4L2_YCBCR_ENC_DEFAULT */
-	enum v4l2_quantization quantization;	/* V4L2_QUANTIZATION_LIM_RANGE */
+	enum v4l2_ycbcr_encoding ycbcr_enc;	/* V4L2_YCBCR_ENC_601 */
+	enum v4l2_quantization quantization;	/* V4L2_QUANTIZATION_FULL_RANGE */
 	enum v4l2_xfer_func xfer_func;	/* V4L2_XFER_FUNC_DEFAULT */
 	bool interlaced;	/* cached hardware state */
 	u32 half_size;		/* hardware half-frame size */
diff --git a/drivers/media/pci/hws/hws_reg.h b/drivers/media/pci/hws/hws_reg.h
index e4fb4af44434..f6027a16a193 100644
--- a/drivers/media/pci/hws/hws_reg.h
+++ b/drivers/media/pci/hws/hws_reg.h
@@ -70,7 +70,8 @@
 #define HWS_CTL_IRQ_ENABLE_BIT BIT(0)   /* Global interrupt enable bit */
 /*  Write 0x00 to fully reset decoder,
  *  set bit 31=1 to "start run",
- *  low byte=0x13 selects YUYV/BT.709/etc,
+ *  low byte=0x13 is the vendor baseline capture-mode value; its individual
+ *  bit meanings are not documented,
  *  in ReadChipId() we also write 0x00 and 0x10 here for chip-ID sequencing.
  */
 
diff --git a/drivers/media/pci/hws/hws_v4l2_ioctl.c b/drivers/media/pci/hws/hws_v4l2_ioctl.c
index 0303e311ee4a..ce396b7225d2 100644
--- a/drivers/media/pci/hws/hws_v4l2_ioctl.c
+++ b/drivers/media/pci/hws/hws_v4l2_ioctl.c
@@ -511,8 +511,9 @@ static inline void hws_set_colorimetry_state(struct hws_pix_state *p)
 {
 	bool sd = p->height <= 576;
 
+	/* Captured samples use full-range BT.601 Y'CbCr at every resolution. */
 	p->colorspace   = sd ? V4L2_COLORSPACE_SMPTE170M : V4L2_COLORSPACE_REC709;
-	p->ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
+	p->ycbcr_enc    = V4L2_YCBCR_ENC_601;
 	p->quantization = V4L2_QUANTIZATION_FULL_RANGE;
 	p->xfer_func    = V4L2_XFER_FUNC_DEFAULT;
 }
@@ -736,7 +737,7 @@ static inline void hws_set_colorimetry_fmt(struct v4l2_pix_format *p)
 	bool sd = p->height <= 576;
 
 	p->colorspace   = sd ? V4L2_COLORSPACE_SMPTE170M : V4L2_COLORSPACE_REC709;
-	p->ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
+	p->ycbcr_enc    = V4L2_YCBCR_ENC_601;
 	p->quantization = V4L2_QUANTIZATION_FULL_RANGE;
 	p->xfer_func    = V4L2_XFER_FUNC_DEFAULT;
 }
diff --git a/drivers/media/pci/hws/hws_video.c b/drivers/media/pci/hws/hws_video.c
index 18e4bc6901d3..09cfb5a909f9 100644
--- a/drivers/media/pci/hws/hws_video.c
+++ b/drivers/media/pci/hws/hws_video.c
@@ -327,7 +327,7 @@ int hws_video_init_channel(struct hws_pcie_dev *pdev, int ch)
 	vid->pix.sizeimage = vid->pix.bytesperline * vid->pix.height;
 	vid->pix.field = V4L2_FIELD_NONE;
 	vid->pix.colorspace = V4L2_COLORSPACE_REC709;
-	vid->pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	vid->pix.ycbcr_enc = V4L2_YCBCR_ENC_601;
 	vid->pix.quantization = V4L2_QUANTIZATION_FULL_RANGE;
 	vid->pix.xfer_func = V4L2_XFER_FUNC_DEFAULT;
 	vid->pix.interlaced = false;
@@ -895,6 +895,8 @@ static void hws_video_apply_mode_change(struct hws_pcie_dev *pdx,
 	v->pix.width = w;
 	v->pix.height = h;
 	v->pix.interlaced = interlaced;
+	v->pix.colorspace = h <= 576 ? V4L2_COLORSPACE_SMPTE170M :
+					 V4L2_COLORSPACE_REC709;
 	hws_set_current_dv_timings(v, w, h, interlaced);
 	v->current_fps = fps;
 
-- 
2.55.0

Re: [PATCH] media: hws: report measured colorimetry

Posted by Ben Hoff 3 weeks, 1 day ago

Hi,

Just following up on this series, originally posted on August 7. I
haven't seen any review comments yet and wanted to check whether
there's anything I should address or whether it's in a good state for
consideration.

Thanks,
Ben

On Fri, Aug 7, 2026 at 3:47 AM Ben Hoff <hoff.benjamin.k@gmail.com> wrote:
>
> At 1920x1080, the driver advertises V4L2_COLORSPACE_REC709 with
> V4L2_YCBCR_ENC_DEFAULT. The default maps to a Rec.709 matrix, but
> controlled A/B captures using the in-tree and baseline drivers show that
> both produce YUYV samples using a BT.601 matrix and full-range
> quantization. The captures differ by at most one code value, whereas a
> 601-to-709 conversion would move primary luma values by roughly 11 to 33
> codes.
>
> Set the Y'CbCr encoding explicitly to V4L2_YCBCR_ENC_601 at every
> resolution. Keep the resolution-based colorspace so HD retains presumed
> Rec.709 primaries and transfer characteristics while SD continues to use
> SMPTE 170M. Refresh the colorspace on detected geometry changes so an
> SD/HD transition does not leave stale metadata.
>
> The decoder mode register is undocumented. Stop claiming that value 0x13
> selects BT.709; both drivers use that value while producing the measured
> output.
>
> Fixes: ba07fd2f5742 ("media: pci: add AVMatrix HWS capture driver")
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Ben Hoff <hoff.benjamin.k@gmail.com>
> ---
>  drivers/media/pci/hws/hws.h            | 4 ++--
>  drivers/media/pci/hws/hws_reg.h        | 3 ++-
>  drivers/media/pci/hws/hws_v4l2_ioctl.c | 5 +++--
>  drivers/media/pci/hws/hws_video.c      | 4 +++-
>  4 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/pci/hws/hws.h b/drivers/media/pci/hws/hws.h
> index 8fbe1fe27844..d87d52674b69 100644
> --- a/drivers/media/pci/hws/hws.h
> +++ b/drivers/media/pci/hws/hws.h
> @@ -33,8 +33,8 @@ struct hws_pix_state {
>         u32 sizeimage;          /* full frame */
>         enum v4l2_field field;  /* V4L2_FIELD_NONE or INTERLACED */
>         enum v4l2_colorspace colorspace;        /* e.g., REC709 */
> -       enum v4l2_ycbcr_encoding ycbcr_enc;     /* V4L2_YCBCR_ENC_DEFAULT */
> -       enum v4l2_quantization quantization;    /* V4L2_QUANTIZATION_LIM_RANGE */
> +       enum v4l2_ycbcr_encoding ycbcr_enc;     /* V4L2_YCBCR_ENC_601 */
> +       enum v4l2_quantization quantization;    /* V4L2_QUANTIZATION_FULL_RANGE */
>         enum v4l2_xfer_func xfer_func;  /* V4L2_XFER_FUNC_DEFAULT */
>         bool interlaced;        /* cached hardware state */
>         u32 half_size;          /* hardware half-frame size */
> diff --git a/drivers/media/pci/hws/hws_reg.h b/drivers/media/pci/hws/hws_reg.h
> index e4fb4af44434..f6027a16a193 100644
> --- a/drivers/media/pci/hws/hws_reg.h
> +++ b/drivers/media/pci/hws/hws_reg.h
> @@ -70,7 +70,8 @@
>  #define HWS_CTL_IRQ_ENABLE_BIT BIT(0)   /* Global interrupt enable bit */
>  /*  Write 0x00 to fully reset decoder,
>   *  set bit 31=1 to "start run",
> - *  low byte=0x13 selects YUYV/BT.709/etc,
> + *  low byte=0x13 is the vendor baseline capture-mode value; its individual
> + *  bit meanings are not documented,
>   *  in ReadChipId() we also write 0x00 and 0x10 here for chip-ID sequencing.
>   */
>
> diff --git a/drivers/media/pci/hws/hws_v4l2_ioctl.c b/drivers/media/pci/hws/hws_v4l2_ioctl.c
> index 0303e311ee4a..ce396b7225d2 100644
> --- a/drivers/media/pci/hws/hws_v4l2_ioctl.c
> +++ b/drivers/media/pci/hws/hws_v4l2_ioctl.c
> @@ -511,8 +511,9 @@ static inline void hws_set_colorimetry_state(struct hws_pix_state *p)
>  {
>         bool sd = p->height <= 576;
>
> +       /* Captured samples use full-range BT.601 Y'CbCr at every resolution. */
>         p->colorspace   = sd ? V4L2_COLORSPACE_SMPTE170M : V4L2_COLORSPACE_REC709;
> -       p->ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
> +       p->ycbcr_enc    = V4L2_YCBCR_ENC_601;
>         p->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>         p->xfer_func    = V4L2_XFER_FUNC_DEFAULT;
>  }
> @@ -736,7 +737,7 @@ static inline void hws_set_colorimetry_fmt(struct v4l2_pix_format *p)
>         bool sd = p->height <= 576;
>
>         p->colorspace   = sd ? V4L2_COLORSPACE_SMPTE170M : V4L2_COLORSPACE_REC709;
> -       p->ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
> +       p->ycbcr_enc    = V4L2_YCBCR_ENC_601;
>         p->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>         p->xfer_func    = V4L2_XFER_FUNC_DEFAULT;
>  }
> diff --git a/drivers/media/pci/hws/hws_video.c b/drivers/media/pci/hws/hws_video.c
> index 18e4bc6901d3..09cfb5a909f9 100644
> --- a/drivers/media/pci/hws/hws_video.c
> +++ b/drivers/media/pci/hws/hws_video.c
> @@ -327,7 +327,7 @@ int hws_video_init_channel(struct hws_pcie_dev *pdev, int ch)
>         vid->pix.sizeimage = vid->pix.bytesperline * vid->pix.height;
>         vid->pix.field = V4L2_FIELD_NONE;
>         vid->pix.colorspace = V4L2_COLORSPACE_REC709;
> -       vid->pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +       vid->pix.ycbcr_enc = V4L2_YCBCR_ENC_601;
>         vid->pix.quantization = V4L2_QUANTIZATION_FULL_RANGE;
>         vid->pix.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>         vid->pix.interlaced = false;
> @@ -895,6 +895,8 @@ static void hws_video_apply_mode_change(struct hws_pcie_dev *pdx,
>         v->pix.width = w;
>         v->pix.height = h;
>         v->pix.interlaced = interlaced;
> +       v->pix.colorspace = h <= 576 ? V4L2_COLORSPACE_SMPTE170M :
> +                                        V4L2_COLORSPACE_REC709;
>         hws_set_current_dv_timings(v, w, h, interlaced);
>         v->current_fps = fps;
>
> --
> 2.55.0
>