Re: [PATCH 3/7] drm/i915: Enable 10bpc + CCS on TGL+

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Nov 25, 2024 at 02:55:34PM +0800, Xi Ruoyao wrote:
> On Tue, 2024-10-08 at 12:01 +0300, Juha-Pekka Heikkila wrote:
> > On 4.10.2024 21.03, Ville Syrjälä wrote:
> > > On Fri, Oct 04, 2024 at 04:35:17PM +0300, Juha-Pekka Heikkila wrote:
> > > > On 18.9.2024 17.44, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > TGL+ support 10bpc compressed scanout. Enable it.
> > > > > 
> > > > > v2: Set .depth=30 for all variants to match drm_fourcc.c
> > > > >       Set clear color block size to 0x0
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > >    drivers/gpu/drm/i915/display/intel_fb.c       | 36 +++++++++++++++++++
> > > > >    .../drm/i915/display/skl_universal_plane.c    |  8 ++---
> > > > >    2 files changed, 40 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> > > > > index bcf0d016f499..9b9da4f71f73 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_fb.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> > > > > @@ -67,6 +67,18 @@ static const struct drm_format_info gen12_ccs_formats[] = {
> > > > >    	{ .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2,
> > > > >    	  .char_per_block = { 4, 1 }, .block_w = { 1, 2 }, .block_h = { 1, 1 },
> > > > >    	  .hsub = 1, .vsub = 1, .has_alpha = true },
> > > > > +	{ .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 2,
> > > > > +	  .char_per_block = { 4, 1 }, .block_w = { 1, 2 }, .block_h = { 1, 1 },
> > > > > +	  .hsub = 1, .vsub = 1, },
> > > > > +	{ .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 2,
> > > > > +	  .char_per_block = { 4, 1 }, .block_w = { 1, 2 }, .block_h = { 1, 1 },
> > > > > +	  .hsub = 1, .vsub = 1, },
> > > > > +	{ .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 2,
> > > > 
> > > > Is that comment about depth=30 for all variants because of these alpha
> > > > formats? Why is that? Here on other formats alpha is taken as part of
> > > > depth, like in above "DRM_FORMAT_ABGR8888, .depth = 32"
> > > 
> > > That stuff is just legacy compatibility stuff, and back in
> > > the day peope decided that depth==32 simply means ARGB8888.
> > > I'm not sure we should even state depth=30 on ARGB2101010
> > > at all, or would it be better to leave it at 0.
> > > 
> > > Another option might be to just set .depth=0 on absolutely
> > > all compressed formats. Using these with some legacy uapi
> > > which only talks in terms of bpp and depth doesn't seem
> > > feasible anyway.
> > > 
> > > But for now I think we just want to match drm_fourcc.c since
> > > that's what we did for the other compressed formats.
> > 
> > ack. patch set is
> > 
> > Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> 
> Hi Ville and Juha-Pekka,
> 
> Unfortunately this commit has caused gnome-shell to display nothing on
> my system.  Its log contains error messages:
> 
>     Failed to ensure KMS FB ID on /dev/dri/card1: drmModeAddFB2WithModifiers failed: Invalid argument
>     meta_frame_native_release: assertion '!frame_native->kms_update' failed
> 
> Reverting commits 7c35015fab5d ("drm/i915: Enable fp16 + CCS on TGL+")
> and c315fbfa44f4 (this one) "fixes" the issue for me.
> 
> The system does have a TGL (i5-11300H) but I don't think my monitor
> (it's just the display panel of a budget laptop) supports 10bpc.
> 
> Any pointer on debugging this further?

Please file a new bug at
https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/new

Boot with 'drm.debug=0x1e log_buf_len=4M' added to the kernel cmdline
and attach the resulting dmesg to the bug.

-- 
Ville Syrjälä
Intel