> Subject: Re: [PATCH 7/7] drm/i915: Allow user to set cache at BO creation > > On 2023-04-05 13:26:43, Jordan Justen wrote: >> On 2023-04-05 00:45:24, Lionel Landwerlin wrote: >>> On 04/04/2023 19:04, Yang, Fei wrote: >>>>> Subject: Re: [PATCH 7/7] drm/i915: Allow user to set >>>>> cache at BO creation >>>>> >>>>> Just like the protected content uAPI, there is no way for >>>>> userspace to tell this feature is available other than trying using it. >>>>> >>>>> Given the issues with protected content, is it not thing we could want to add? >>>> Sorry I'm not aware of the issues with protected content, could you elaborate? >>>> There was a long discussion on teams uAPI channel, could you >>>> comment there if any concerns? >>>> >>> >>> We wanted to have a getparam to detect protected support and were >>> told to detect it by trying to create a context with it. >>> >> >> An extensions system where the detection mechanism is "just try it", >> and assume it's not supported if it fails. ?? >> > > I guess no one wants to discuss the issues with this so-called detection > mechanism for i915 extensions. (Just try it and if it fails, it must not be supported.) > > I wonder how many ioctls we will be making a couple years down the road > just to see what the kernel supports. > > Maybe we'll get more fun 8 second timeouts to deal with. Maybe these probing > ioctls failing or succeeding will alter the kmd's state in some unexpected way. For this SET_PAT extension, I can assure you there is no 8 second wait :) This is definitely a non-blocking call. > It'll also be fun to debug cases where the driver is not starting up with the > noise of a bunch of probing ioctls flying by. > > I thought about suggesting at least something like I915_PARAM_CMD_PARSER_VERSION, > but I don't know if that could have prevented this 8 second timeout for creating > a protected content context. Maybe it's better than nothing though. > > Of course, there was also the vague idea I threw out below for getting a list of > supported extentions. The detection mechanism itself is an uAPI change, I don't think it's a good idea to combine that with this SET_PAT extension patch. I suggest we start a discussion in the "i915 uAPI changes" teams channel just like how we sorted out a solution for this setting cache policy issue. Would that work? https://teams.microsoft.com/l/channel/19%3af1767bda6734476ba0a9c7d147b928d1%40thread.skype/i915%2520uAPI%2520changes?groupId=379f3ae1-d138-4205-bb65-d4c7d38cb481&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d -Fei > -Jordan > >> >> This seem likely to get more and more problematic as a detection >> mechanism as more extensions are added. >> >> > >> > Now it appears trying to create a protected context can block for >> > several seconds. >> > >> > Since we have to report capabilities to the user even before it >> > creates protected contexts, any app is at risk of blocking. >> > >> >> This failure path is not causing any re-thinking about using this as >> the extension detection mechanism? >> >> Doesn't the ioctl# + input-struct-size + u64-extension# identify the >> extension such that the kernel could indicate if it is supported or >> not. (Or, perhaps return an array of the supported extensions so the >> umd doesn't have to potentially make many ioctls for each extension of >> interest.) >> >> -Jordan
