On 02.07.2021 15:12, Martin Peres wrote: > On 02/07/2021 16:06, Michal Wajdeczko wrote: >> >> >> On 02.07.2021 10:13, Martin Peres wrote: >>> On 01/07/2021 21:24, Martin Peres wrote: >>> [...] >>>>> >>>>>> >>>>>>> + i915->params.enable_guc = ENABLE_GUC_LOAD_HUC; >>>>>>> + return; >>>>>>> + } >>>>>>> + >>>>>>> + /* Default: enable HuC authentication and GuC submission */ >>>>>>> + i915->params.enable_guc = ENABLE_GUC_LOAD_HUC | >>>>>>> ENABLE_GUC_SUBMISSION; >>>>>> >>>>>> This seems to be in contradiction with the GuC submission plan which >>>>>> states: >>>>>> >>>>>> "Not enabled by default on any current platforms but can be >>>>>> enabled via >>>>>> modparam enable_guc". >>>>>> >>>>> >>>>> I don't believe any current platform gets this point where GuC >>>>> submission would be enabled by default. The first would be ADL-P which >>>>> isn't out yet. >>>> >>>> Isn't that exactly what the line above does? >>> >>> In case you missed this crucial part of the review. Please answer the >>> above question. >> >> I guess there is some misunderstanding here, and I must admit I had >> similar doubt, but if you look beyond patch diff and check function code >> you will find that the very condition is: >> >> /* Don't enable GuC/HuC on pre-Gen12 */ >> if (GRAPHICS_VER(i915) < 12) { >> i915->params.enable_guc = 0; >> return; >> } >> >> so all pre-Gen12 platforms will continue to have GuC/HuC disabled. > > Thanks Michal, but then the problem is the other way: how can one enable > it on gen11? this code here converts default GuC auto mode (enable_guc=-1) into per platform desired (tested) GuC/HuC enables. to override that default, you may still use enable_guc=1 to explicitly enable GuC submission and since we also have this code: +static bool __guc_submission_supported(struct intel_guc *guc) +{ + /* GuC submission is unavailable for pre-Gen11 */ + return intel_guc_is_supported(guc) && + INTEL_GEN(guc_to_gt(guc)->i915) >= 11; +} it should work on any Gen11+. Michal > > I like what Daniele was going for here: separating the capability from > the user-requested value, but then it seems the patch stopped half way. > How about never touching the parameter, and having a AND between the two > values to get the effective enable_guc? > > Right now, the code is really confusing :s > > Thanks, > Martin > >> >> Thanks, >> Michal >> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx