Re: [PATCH v2 1/3] drm: Add the optional .fb_modifier() hook

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 1/3] drm: Add the optional .fb_modifier() hook
Date: Tue, 13 Mar 2018 17:46:57 +0200	[thread overview]
Message-ID: <20180313154657.GA5453@intel.com> (raw)
In-Reply-To: <108f283c-37bd-6c47-40d0-236f839cd021@daenzer.net>

On Tue, Mar 13, 2018 at 04:35:02PM +0100, Michel Dänzer wrote:
> On 2018-03-13 04:20 PM, Daniel Vetter wrote:
> > On Tue, Mar 13, 2018 at 03:38:38PM +0100, Michel Dänzer wrote:
> >> On 2018-03-13 03:28 PM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> To make it possible for the core to check the fb pixel format and
> >>> modifier, we need to first ask the driver to deduce the modifier
> >>> when the request does not explicitly specify one.
> >>>
> >>> Add a new .fb_modifier() hook for that purpose and convert i915
> >>> and vc4 to make use if it. All other drivers seem to currently
> >>> assume linear when the request does not specify anything else,
> >>> [...]
> >>
> >> That's not true at least for the amdgpu and radeon drivers. The tiling
> >> mode is communicated via BO metadata.
> > 
> > But atm amdgpu and radeon also don't support explicit modifiers in the
> > kernel driver, so it again all checks out. Or should at least.
> 
> Sounds like I misunderstood that this is trying to guess modifiers for
> all drivers. So far, so good if so.

Somehow I convinced myself that amdgpu already had some modifier
support. But looks like I was mistaken.

> > Once you add modifier support, you need to wire up all the bits and the
> > rigth default selection, and it should again pan out.
> 
> Well, this change allows a driver not to wire up the fb_modifier hook,
> and just assumes linear in that case. Seems like an accident waiting to
> happen, but I'll leave it to you guys.

That's already the case with the current code. mode_cmd->modifier[0]
will be 0 when when DRM_MODE_FB_MODIFIERS isn't specified, which is the
same thing as DRM_FORMAT_MOD_LINEAR. So if the driver doesn't change
it before calling drm_helper_mode_fill_fb_struct() you end up with
fb->modifier == DRM_FORMAT_MOD_LINEAR.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel