| Message ID | 20240226211100.100108-24-harry.wentland@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Color Pipeline API w/ VKMS | expand |
On Mon, 26 Feb 2024 16:10:37 -0500 Harry Wentland <harry.wentland@amd.com> wrote: > We add two 3x4 matrices into the VKMS color pipeline. The reason > we're adding matrices is so that we can test that application > of a matrix and its inverse yields an output equal to the input > image. You will test also cases where the matrix configuration will not produce output equal to input, right? Otherwise one can accidentally pass the matrix test by ignoring all matrices. > One complication with the matrix implementation has to do with > the fact that the matrix entries are in signed-magnitude fixed > point, whereas the drm_fixed.h implementation uses 2s-complement. > The latter one is the one that we want for easy addition and > subtraction, so we convert all entries to 2s-complement. > > Signed-off-by: Harry Wentland <harry.wentland@amd.com> > --- > drivers/gpu/drm/vkms/vkms_colorop.c | 32 +++++++++++++++++++++++++++- > drivers/gpu/drm/vkms/vkms_composer.c | 27 +++++++++++++++++++++++ > include/drm/drm_colorop.h | 2 ++ > 3 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c > index d2db366da6d3..a0e54b2c1f7a 100644 > --- a/drivers/gpu/drm/vkms/vkms_colorop.c > +++ b/drivers/gpu/drm/vkms/vkms_colorop.c > @@ -35,7 +35,37 @@ const int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pro > > prev_op = op; > > - /* 2nd op: 1d curve */ > + /* 2nd op: 3x4 matrix */ > + op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL); > + if (!op) { > + DRM_ERROR("KMS: Failed to allocate colorop\n"); > + return -ENOMEM; > + } > + > + ret = drm_colorop_ctm_3x4_init(dev, op, plane); > + if (ret) > + return ret; > + > + drm_colorop_set_next_property(prev_op, op); > + > + prev_op = op; > + > + /* 3rd op: 3x4 matrix */ > + op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL); > + if (!op) { > + DRM_ERROR("KMS: Failed to allocate colorop\n"); > + return -ENOMEM; > + } > + > + ret = drm_colorop_ctm_3x4_init(dev, op, plane); > + if (ret) > + return ret; > + > + drm_colorop_set_next_property(prev_op, op); > + > + prev_op = op; > + > + /* 4th op: 1d curve */ > op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL); > if (!op) { > DRM_ERROR("KMS: Failed to allocate colorop\n"); > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c > index d2101fa55aa3..8bbfce651526 100644 > --- a/drivers/gpu/drm/vkms/vkms_composer.c > +++ b/drivers/gpu/drm/vkms/vkms_composer.c > @@ -164,6 +164,30 @@ static void apply_lut(const struct vkms_crtc_state *crtc_state, struct line_buff > } > } > > +static void apply_3x4_matrix(struct pixel_argb_s32 *pixel, const struct drm_color_ctm_3x4 *matrix) > +{ > + s64 rf, gf, bf; > + > + rf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[0]), drm_int2fixp(pixel->r)) + > + drm_fixp_mul(drm_sm2fixp(matrix->matrix[1]), drm_int2fixp(pixel->g)) + > + drm_fixp_mul(drm_sm2fixp(matrix->matrix[2]), drm_int2fixp(pixel->b)) + > + drm_sm2fixp(matrix->matrix[3]); It would be nice if the driver had its private data for the matrix colorop state, so it could convert the matrix only once when userspace sets it. Thanks, pq > + > + gf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[4]), drm_int2fixp(pixel->r)) + > + drm_fixp_mul(drm_sm2fixp(matrix->matrix[5]), drm_int2fixp(pixel->g)) + > + drm_fixp_mul(drm_sm2fixp(matrix->matrix[6]), drm_int2fixp(pixel->b)) + > + drm_sm2fixp(matrix->matrix[7]); > + > + bf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[8]), drm_int2fixp(pixel->r)) + > + drm_fixp_mul(drm_sm2fixp(matrix->matrix[9]), drm_int2fixp(pixel->g)) + > + drm_fixp_mul(drm_sm2fixp(matrix->matrix[10]), drm_int2fixp(pixel->b)) + > + drm_sm2fixp(matrix->matrix[11]); > + > + pixel->r = drm_fixp2int(rf); > + pixel->g = drm_fixp2int(gf); > + pixel->b = drm_fixp2int(bf); > +} > + > static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop *colorop) > { > /* TODO is this right? */ > @@ -185,6 +209,9 @@ static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop *colo > DRM_DEBUG_DRIVER("unkown colorop 1D curve type %d\n", colorop_state->curve_1d_type); > break; > } > + } else if (colorop->type == DRM_COLOROP_CTM_3X4) { > + if (colorop_state->data) > + apply_3x4_matrix(pixel, (struct drm_color_ctm_3x4 *) colorop_state->data->data); > } > > } > diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h > index 4aee29e161d6..8710e550790c 100644 > --- a/include/drm/drm_colorop.h > +++ b/include/drm/drm_colorop.h > @@ -224,6 +224,8 @@ int drm_colorop_init(struct drm_device *dev, struct drm_colorop *colorop, > > int drm_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop, > struct drm_plane *plane, u64 supported_tfs); > +int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop, > + struct drm_plane *plane); > > struct drm_colorop_state * > drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop);
diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c b/drivers/gpu/drm/vkms/vkms_colorop.c index d2db366da6d3..a0e54b2c1f7a 100644 --- a/drivers/gpu/drm/vkms/vkms_colorop.c +++ b/drivers/gpu/drm/vkms/vkms_colorop.c @@ -35,7 +35,37 @@ const int vkms_initialize_color_pipeline(struct drm_plane *plane, struct drm_pro prev_op = op; - /* 2nd op: 1d curve */ + /* 2nd op: 3x4 matrix */ + op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL); + if (!op) { + DRM_ERROR("KMS: Failed to allocate colorop\n"); + return -ENOMEM; + } + + ret = drm_colorop_ctm_3x4_init(dev, op, plane); + if (ret) + return ret; + + drm_colorop_set_next_property(prev_op, op); + + prev_op = op; + + /* 3rd op: 3x4 matrix */ + op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL); + if (!op) { + DRM_ERROR("KMS: Failed to allocate colorop\n"); + return -ENOMEM; + } + + ret = drm_colorop_ctm_3x4_init(dev, op, plane); + if (ret) + return ret; + + drm_colorop_set_next_property(prev_op, op); + + prev_op = op; + + /* 4th op: 1d curve */ op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL); if (!op) { DRM_ERROR("KMS: Failed to allocate colorop\n"); diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c index d2101fa55aa3..8bbfce651526 100644 --- a/drivers/gpu/drm/vkms/vkms_composer.c +++ b/drivers/gpu/drm/vkms/vkms_composer.c @@ -164,6 +164,30 @@ static void apply_lut(const struct vkms_crtc_state *crtc_state, struct line_buff } } +static void apply_3x4_matrix(struct pixel_argb_s32 *pixel, const struct drm_color_ctm_3x4 *matrix) +{ + s64 rf, gf, bf; + + rf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[0]), drm_int2fixp(pixel->r)) + + drm_fixp_mul(drm_sm2fixp(matrix->matrix[1]), drm_int2fixp(pixel->g)) + + drm_fixp_mul(drm_sm2fixp(matrix->matrix[2]), drm_int2fixp(pixel->b)) + + drm_sm2fixp(matrix->matrix[3]); + + gf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[4]), drm_int2fixp(pixel->r)) + + drm_fixp_mul(drm_sm2fixp(matrix->matrix[5]), drm_int2fixp(pixel->g)) + + drm_fixp_mul(drm_sm2fixp(matrix->matrix[6]), drm_int2fixp(pixel->b)) + + drm_sm2fixp(matrix->matrix[7]); + + bf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[8]), drm_int2fixp(pixel->r)) + + drm_fixp_mul(drm_sm2fixp(matrix->matrix[9]), drm_int2fixp(pixel->g)) + + drm_fixp_mul(drm_sm2fixp(matrix->matrix[10]), drm_int2fixp(pixel->b)) + + drm_sm2fixp(matrix->matrix[11]); + + pixel->r = drm_fixp2int(rf); + pixel->g = drm_fixp2int(gf); + pixel->b = drm_fixp2int(bf); +} + static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop *colorop) { /* TODO is this right? */ @@ -185,6 +209,9 @@ static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop *colo DRM_DEBUG_DRIVER("unkown colorop 1D curve type %d\n", colorop_state->curve_1d_type); break; } + } else if (colorop->type == DRM_COLOROP_CTM_3X4) { + if (colorop_state->data) + apply_3x4_matrix(pixel, (struct drm_color_ctm_3x4 *) colorop_state->data->data); } } diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index 4aee29e161d6..8710e550790c 100644 --- a/include/drm/drm_colorop.h +++ b/include/drm/drm_colorop.h @@ -224,6 +224,8 @@ int drm_colorop_init(struct drm_device *dev, struct drm_colorop *colorop, int drm_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop *colorop, struct drm_plane *plane, u64 supported_tfs); +int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop *colorop, + struct drm_plane *plane); struct drm_colorop_state * drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop);
We add two 3x4 matrices into the VKMS color pipeline. The reason we're adding matrices is so that we can test that application of a matrix and its inverse yields an output equal to the input image. One complication with the matrix implementation has to do with the fact that the matrix entries are in signed-magnitude fixed point, whereas the drm_fixed.h implementation uses 2s-complement. The latter one is the one that we want for easy addition and subtraction, so we convert all entries to 2s-complement. Signed-off-by: Harry Wentland <harry.wentland@amd.com> --- drivers/gpu/drm/vkms/vkms_colorop.c | 32 +++++++++++++++++++++++++++- drivers/gpu/drm/vkms/vkms_composer.c | 27 +++++++++++++++++++++++ include/drm/drm_colorop.h | 2 ++ 3 files changed, 60 insertions(+), 1 deletion(-)