On Wed, Jun 05, 2024 at 11:34:57AM +0530, Melanie Lobo wrote: > Add support for a RGB64(FP16) format where each color component is a > 16-bit floating point value. FP16 format which is a binary > floating-point computer number format that occupies 16 bits in computer > memory. Platform shall render compression in display engine to receive > FP16 compressed formats. > > This kernel change was tested with IGT patch, > https://patchwork.freedesktop.org/series/134353/ > https://lore.kernel.org/all/20240603081607.30930-1-melanie.lobo@xxxxxxxxx/ > > Test-with: 20240603081607.30930-1-melanie.lobo@xxxxxxxxx > > Credits: Juha-Pekka <juha-pekka.heikkila@xxxxxxxxx> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@xxxxxxxxx> > Cc: Bhanuprakash Modem <bhanuprakash.modem@xxxxxxxxx> > Cc: Swati Sharma <swati2.sharma@xxxxxxxxx> > Signed-off-by: Melanie Lobo <melanie.lobo@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_fb.c | 5 +++++ > drivers/gpu/drm/i915/display/skl_universal_plane.c | 4 ++-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c > index b6638726949d..91f2def14243 100644 > --- a/drivers/gpu/drm/i915/display/intel_fb.c > +++ b/drivers/gpu/drm/i915/display/intel_fb.c > @@ -91,6 +91,11 @@ static const struct drm_format_info gen12_ccs_formats[] = { > { .format = DRM_FORMAT_P016, .num_planes = 4, > .char_per_block = { 2, 4, 1, 1 }, .block_w = { 1, 1, 2, 2 }, .block_h = { 1, 1, 1, 1 }, > .hsub = 2, .vsub = 2, .is_yuv = true }, I'd put the fp16 formats between the other RGB formats and YCbCr formats. > + { .format = DRM_FORMAT_XRGB16161616F, .depth = 64, .num_planes = 2, __drm_format_info() leaves depth==0 for fp16 formats. These should do the same. > + .char_per_block = { 8, 1}, .block_w = { 1, 4}, .block_h = { 1, 2}, .hsub = 1, .vsub = 1}, Everyone else wraps the hsub/vsub to the next line. Also you consistently leave out the space before '}', unlike every other line in this file. > + { .format = DRM_FORMAT_ARGB16161616F, .depth = 64, .num_planes = 2, > + .char_per_block = { 8, 1}, .block_w = { 1, 4}, .block_h = { 1, 2}, AUX block width should be 1 for these, block height we don't actually use I believe, but everone else sets it to one so this should do the same. > + .hsub = 1, .vsub = 1, .has_alpha = true}, You are missing the X/ABGR variants. Also missing the addition of these formats to gen12_ccs_cc_formats[] and gen12_flat_ccs_cc_formats[]. See my equivalent patch for the 10bpc formats: https://patchwork.freedesktop.org/patch/600652/?series=135306&rev=1 > }; > > /* > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index 1aa70fc35b9d..7719cb04bdf8 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -2242,6 +2242,8 @@ static bool gen12_plane_format_mod_supported(struct drm_plane *_plane, > case DRM_FORMAT_XBGR8888: > case DRM_FORMAT_ARGB8888: > case DRM_FORMAT_ABGR8888: > + case DRM_FORMAT_XRGB16161616F: > + case DRM_FORMAT_ARGB16161616F: > if (intel_fb_is_ccs_modifier(modifier)) > return true; > fallthrough; > @@ -2266,8 +2268,6 @@ static bool gen12_plane_format_mod_supported(struct drm_plane *_plane, > case DRM_FORMAT_C8: > case DRM_FORMAT_XBGR16161616F: > case DRM_FORMAT_ABGR16161616F: > - case DRM_FORMAT_XRGB16161616F: > - case DRM_FORMAT_ARGB16161616F: > case DRM_FORMAT_Y210: > case DRM_FORMAT_Y212: > case DRM_FORMAT_Y216: > -- > 2.17.1 -- Ville Syrjälä Intel