On Thu, May 17, 2018 at 11:55:47AM +0100, Ayan Halder wrote:
> Hi,
>
> I was going through drivers/gpu/drm/i915/intel_display.c to get an
> understanding about how framebuffer modifiers are used in the drm
> subsystem.
> I could see the following in intel_framebuffer_init(),
> (added in commit 2e2adb0573)
>
> << some code >>
> switch (mode_cmd->modifier[0]) {
> case I915_FORMAT_MOD_Y_TILED_CCS:
> case I915_FORMAT_MOD_Yf_TILED_CCS:
> switch (mode_cmd->pixel_format) {
> case DRM_FORMAT_XBGR8888:
> case DRM_FORMAT_ABGR8888:
> case DRM_FORMAT_XRGB8888:
> case DRM_FORMAT_ARGB8888:
> break;
> default:
> DRM_DEBUG_KMS("RC supported only with RGB8888 formats\n");
> goto err;
> }
> << some code >>
>
> And I see the following intel_primary_plane_format_mod_supported() -->
> skl_mod_supported()
> (added in commit 714244e280)
>
> << some code >>
> switch (format) {
> case DRM_FORMAT_XRGB8888:
> case DRM_FORMAT_XBGR8888:
> case DRM_FORMAT_ARGB8888:
> case DRM_FORMAT_ABGR8888:
> if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> modifier == I915_FORMAT_MOD_Y_TILED_CCS)
> return true;
> << some code >>
>
> Is it a case of duplicacy? If we are checking for valid
> combination of formats and modifiers in intel_framebuffer_init(), then why do we
> need to check it again in skl_mod_supported()?
>
> Can we just check the valid combination only in
> skl_mod_supported() and not in intel_framebuffer_init() ?
The check in intel_framebuffer_init() is there to prevent the creation
of framebuffers that can't be scanned out by any plane. In theory we
don't have to do that, but I think it's a good idea in case userspace
just blindly probes which format+modifier combos are supported. And in
fact, before we got the IN_FORMATS property on planes blind probing
was the only way to discover the supported modifiers.
That said, I do want to eliminate that code from
intel_framebuffer_init() and replace it with a piece of generic code
that simply goes through all the planes and checks whether any of
them support the requested format+modifier combo. I've posted some
patches for that, but there were some unforseen complications due
to how some other drivers want to use modifiers. The last patch
I posted on the topic https://patchwork.freedesktop.org/patch/211306/
should help us proceed but it didn't get reviewed so we're stuck
until I can nudge it forward somehow.
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
