Hi,
On 28/03/2025 00:52, Laurent Pinchart wrote:
Hi Tomi,
Thank you for the patch.
On Wed, Mar 26, 2025 at 03:22:52PM +0200, Tomi Valkeinen wrote:
Add support for Y8 and Y10_P32 formats. We also need to add new csc
matrices for the y-only formats.
Signed-off-by: Tomi Valkeinen <tomi.valkei...@ideasonboard.com>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 1dc77f2e4262..ae8b4073edf6 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -307,6 +307,16 @@ static const struct zynqmp_disp_format avbuf_vid_fmts[] = {
.buf_fmt = ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV16CI_10,
.swap = false,
.sf = scaling_factors_101010,
+ }, {
+ .drm_fmt = DRM_FORMAT_Y8,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MONO,
+ .swap = false,
+ .sf = scaling_factors_888,
+ }, {
+ .drm_fmt = DRM_FORMAT_Y10_P32,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YONLY_10,
+ .swap = false,
+ .sf = scaling_factors_101010,
Assuming the DRM format definitions get approved, this looks good to me.
},
};
@@ -697,6 +707,16 @@ static const u32 csc_sdtv_to_rgb_offsets[] = {
0x0, 0x1800, 0x1800
};
+static const u16 csc_sdtv_to_rgb_yonly_matrix[] = {
TODO: Add support for colorspaces to the driver.
+ 0x0, 0x0, 0x1000,
+ 0x0, 0x0, 0x1000,
+ 0x0, 0x0, 0x1000,
This surprises me a bit, I was expecting 0x1000 to be in the first
column. What am I missing ?
All this is undocumented (afaics). But my understanding is that as this
is a single channel format, the Y data is in the "lowest" channel, which
is handled by the rightmost column in the matrix.
+};
+
+static const u32 csc_sdtv_to_rgb_yonly_offsets[] = {
+ 0x1800, 0x1800, 0x0
Why do you need offsets ? Those values correspond to -128 in a 8-bit
range, and that's what would need to be applied to the chroma values.
There's no chroma here. I think you could use csc_zero_offsets.
Indeed, the values are not needed. The only value in the offsets that
matters is the last one (matching the rightmost column in the matrix),
which needs to be 0. But I think it makes sense to have the array here,
otherwise one needs to dig into the code to see what are the offsets for
y-only.
Tomi
