| Message ID | 20230630170140.17319-2-nirmoy.das@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/gt: Do not use stolen on MTL | expand |
On 30.06.2023 19:01, Nirmoy Das wrote: > Use smem on MTL due to a HW bug in MTL that prevents > reading from stolen memory using LMEM BAR. > > Cc: Oak Zeng <oak.zeng@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ > drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c > index 1cc0ddc6a310..10e38d60f9ef 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c > @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper, > obj = i915_gem_object_create_lmem(dev_priv, size, > I915_BO_ALLOC_CONTIGUOUS | > I915_BO_ALLOC_USER); > + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ > + obj = i915_gem_object_create_shmem(dev_priv, size); If you put the check inside following else clause, you will have only one place to call i915_gem_object_create_shmem. > } else { > /* > * If the FB is too big, just don't use it since fbdev is not very > diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c > index d6fe2bbabe55..05ae446c8a56 100644 > --- a/drivers/gpu/drm/i915/display/intel_overlay.c > +++ b/drivers/gpu/drm/i915/display/intel_overlay.c > @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data, > static int get_registers(struct intel_overlay *overlay, bool use_phys) > { > struct drm_i915_private *i915 = overlay->i915; > - struct drm_i915_gem_object *obj; > + struct drm_i915_gem_object *obj = NULL; > struct i915_vma *vma; > int err; > > - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); > - if (IS_ERR(obj)) > + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ > + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); > + if (IS_ERR_OR_NULL(obj)) Initializing obj with ERR_PTR(-ENODEV) and using IS_ERR here will be aligned to previous stanza (intelfb_alloc). Since my comments are close to bikeshedding, please go your way if you wish: Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > obj = i915_gem_object_create_internal(i915, PAGE_SIZE); > if (IS_ERR(obj)) > return PTR_ERR(obj);
On 30/06/2023 18:01, Nirmoy Das wrote: > Use smem on MTL due to a HW bug in MTL that prevents > reading from stolen memory using LMEM BAR. Does anything remain in stolen or could the memory region just not be created? Regards, Tvrtko > Cc: Oak Zeng <oak.zeng@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ > drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c > index 1cc0ddc6a310..10e38d60f9ef 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c > @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper, > obj = i915_gem_object_create_lmem(dev_priv, size, > I915_BO_ALLOC_CONTIGUOUS | > I915_BO_ALLOC_USER); > + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ > + obj = i915_gem_object_create_shmem(dev_priv, size); > } else { > /* > * If the FB is too big, just don't use it since fbdev is not very > diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c > index d6fe2bbabe55..05ae446c8a56 100644 > --- a/drivers/gpu/drm/i915/display/intel_overlay.c > +++ b/drivers/gpu/drm/i915/display/intel_overlay.c > @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data, > static int get_registers(struct intel_overlay *overlay, bool use_phys) > { > struct drm_i915_private *i915 = overlay->i915; > - struct drm_i915_gem_object *obj; > + struct drm_i915_gem_object *obj = NULL; > struct i915_vma *vma; > int err; > > - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); > - if (IS_ERR(obj)) > + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ > + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); > + if (IS_ERR_OR_NULL(obj)) > obj = i915_gem_object_create_internal(i915, PAGE_SIZE); > if (IS_ERR(obj)) > return PTR_ERR(obj);
On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote: > > On 30/06/2023 18:01, Nirmoy Das wrote: >> Use smem on MTL due to a HW bug in MTL that prevents >> reading from stolen memory using LMEM BAR. > > Does anything remain in stolen or could the memory region just not be > created? GSC requires DSM which can't use smem for another bug. Regards, Nirmoy > > Regards, > > Tvrtko > >> Cc: Oak Zeng <oak.zeng@intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Andi Shyti <andi.shyti@linux.intel.com> >> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ >> drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- >> 2 files changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c >> b/drivers/gpu/drm/i915/display/intel_fbdev.c >> index 1cc0ddc6a310..10e38d60f9ef 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >> @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper >> *helper, >> obj = i915_gem_object_create_lmem(dev_priv, size, >> I915_BO_ALLOC_CONTIGUOUS | >> I915_BO_ALLOC_USER); >> + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ >> + obj = i915_gem_object_create_shmem(dev_priv, size); >> } else { >> /* >> * If the FB is too big, just don't use it since fbdev is >> not very >> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c >> b/drivers/gpu/drm/i915/display/intel_overlay.c >> index d6fe2bbabe55..05ae446c8a56 100644 >> --- a/drivers/gpu/drm/i915/display/intel_overlay.c >> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c >> @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct >> drm_device *dev, void *data, >> static int get_registers(struct intel_overlay *overlay, bool use_phys) >> { >> struct drm_i915_private *i915 = overlay->i915; >> - struct drm_i915_gem_object *obj; >> + struct drm_i915_gem_object *obj = NULL; >> struct i915_vma *vma; >> int err; >> - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >> - if (IS_ERR(obj)) >> + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ >> + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >> + if (IS_ERR_OR_NULL(obj)) >> obj = i915_gem_object_create_internal(i915, PAGE_SIZE); >> if (IS_ERR(obj)) >> return PTR_ERR(obj);
On 06/07/2023 14:35, Nirmoy Das wrote: > > On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote: >> >> On 30/06/2023 18:01, Nirmoy Das wrote: >>> Use smem on MTL due to a HW bug in MTL that prevents >>> reading from stolen memory using LMEM BAR. >> >> Does anything remain in stolen or could the memory region just not be >> created? > > > GSC requires DSM which can't use smem for another bug. Okay, thanks. As a related comment, these if-if-if object creation ladders were always a bit ugly and some years ago I was suggesting we create a helper with some "intent/usage" flags. Which could then dtrt ie. create the right object for that intent/usage and platform. I *think* I possibly even had a RFC... need to try and find it. Regards, Tvrtko > > Regards, > > Nirmoy > > >> >> Regards, >> >> Tvrtko >> >>> Cc: Oak Zeng <oak.zeng@intel.com> >>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >>> Cc: Andi Shyti <andi.shyti@linux.intel.com> >>> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ >>> drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- >>> 2 files changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c >>> b/drivers/gpu/drm/i915/display/intel_fbdev.c >>> index 1cc0ddc6a310..10e38d60f9ef 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >>> @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper >>> *helper, >>> obj = i915_gem_object_create_lmem(dev_priv, size, >>> I915_BO_ALLOC_CONTIGUOUS | >>> I915_BO_ALLOC_USER); >>> + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ >>> + obj = i915_gem_object_create_shmem(dev_priv, size); >>> } else { >>> /* >>> * If the FB is too big, just don't use it since fbdev is >>> not very >>> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c >>> b/drivers/gpu/drm/i915/display/intel_overlay.c >>> index d6fe2bbabe55..05ae446c8a56 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_overlay.c >>> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c >>> @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct >>> drm_device *dev, void *data, >>> static int get_registers(struct intel_overlay *overlay, bool use_phys) >>> { >>> struct drm_i915_private *i915 = overlay->i915; >>> - struct drm_i915_gem_object *obj; >>> + struct drm_i915_gem_object *obj = NULL; >>> struct i915_vma *vma; >>> int err; >>> - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>> - if (IS_ERR(obj)) >>> + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ >>> + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>> + if (IS_ERR_OR_NULL(obj)) >>> obj = i915_gem_object_create_internal(i915, PAGE_SIZE); >>> if (IS_ERR(obj)) >>> return PTR_ERR(obj);
Hi Tvrkto, On 7/6/2023 3:43 PM, Tvrtko Ursulin wrote: > > On 06/07/2023 14:35, Nirmoy Das wrote: >> >> On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote: >>> >>> On 30/06/2023 18:01, Nirmoy Das wrote: >>>> Use smem on MTL due to a HW bug in MTL that prevents >>>> reading from stolen memory using LMEM BAR. >>> >>> Does anything remain in stolen or could the memory region just not >>> be created? >> >> >> GSC requires DSM which can't use smem for another bug. > > Okay, thanks. > > As a related comment, these if-if-if object creation ladders were > always a bit ugly and some years ago I was suggesting we create a > helper with some "intent/usage" flags. Which could then dtrt ie. > create the right object for that intent/usage and platform. I *think* > I possibly even had a RFC... need to try and find it. Did you find it :) Would be nice to have a better way to detect and apply memory region as per platfrom/usecase. Regards, Nirmoy > > Regards, > > Tvrtko > >> >> Regards, >> >> Nirmoy >> >> >>> >>> Regards, >>> >>> Tvrtko >>> >>>> Cc: Oak Zeng <oak.zeng@intel.com> >>>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >>>> Cc: Andi Shyti <andi.shyti@linux.intel.com> >>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ >>>> drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- >>>> 2 files changed, 6 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c >>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c >>>> index 1cc0ddc6a310..10e38d60f9ef 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >>>> @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper >>>> *helper, >>>> obj = i915_gem_object_create_lmem(dev_priv, size, >>>> I915_BO_ALLOC_CONTIGUOUS | >>>> I915_BO_ALLOC_USER); >>>> + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ >>>> + obj = i915_gem_object_create_shmem(dev_priv, size); >>>> } else { >>>> /* >>>> * If the FB is too big, just don't use it since fbdev is >>>> not very >>>> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c >>>> b/drivers/gpu/drm/i915/display/intel_overlay.c >>>> index d6fe2bbabe55..05ae446c8a56 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_overlay.c >>>> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c >>>> @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct >>>> drm_device *dev, void *data, >>>> static int get_registers(struct intel_overlay *overlay, bool >>>> use_phys) >>>> { >>>> struct drm_i915_private *i915 = overlay->i915; >>>> - struct drm_i915_gem_object *obj; >>>> + struct drm_i915_gem_object *obj = NULL; >>>> struct i915_vma *vma; >>>> int err; >>>> - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>>> - if (IS_ERR(obj)) >>>> + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ >>>> + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>>> + if (IS_ERR_OR_NULL(obj)) >>>> obj = i915_gem_object_create_internal(i915, PAGE_SIZE); >>>> if (IS_ERR(obj)) >>>> return PTR_ERR(obj);
On 7/5/2023 12:02 PM, Andrzej Hajda wrote: > > > On 30.06.2023 19:01, Nirmoy Das wrote: >> Use smem on MTL due to a HW bug in MTL that prevents >> reading from stolen memory using LMEM BAR. >> >> Cc: Oak Zeng <oak.zeng@intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Andi Shyti <andi.shyti@linux.intel.com> >> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ >> drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- >> 2 files changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c >> b/drivers/gpu/drm/i915/display/intel_fbdev.c >> index 1cc0ddc6a310..10e38d60f9ef 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >> @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper >> *helper, >> obj = i915_gem_object_create_lmem(dev_priv, size, >> I915_BO_ALLOC_CONTIGUOUS | >> I915_BO_ALLOC_USER); >> + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ >> + obj = i915_gem_object_create_shmem(dev_priv, size); > > If you put the check inside following else clause, you will have only > one place to call i915_gem_object_create_shmem. > >> } else { >> /* >> * If the FB is too big, just don't use it since fbdev is >> not very >> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c >> b/drivers/gpu/drm/i915/display/intel_overlay.c >> index d6fe2bbabe55..05ae446c8a56 100644 >> --- a/drivers/gpu/drm/i915/display/intel_overlay.c >> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c >> @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct >> drm_device *dev, void *data, >> static int get_registers(struct intel_overlay *overlay, bool use_phys) >> { >> struct drm_i915_private *i915 = overlay->i915; >> - struct drm_i915_gem_object *obj; >> + struct drm_i915_gem_object *obj = NULL; >> struct i915_vma *vma; >> int err; >> - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >> - if (IS_ERR(obj)) >> + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ >> + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >> + if (IS_ERR_OR_NULL(obj)) > > Initializing obj with ERR_PTR(-ENODEV) and using IS_ERR here will be > aligned to previous stanza (intelfb_alloc). > > Since my comments are close to bikeshedding, I will resend with those suggestion. Thanks, Nirmoy > please go your way if you wish: > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> > > Regards > Andrzej > >> obj = i915_gem_object_create_internal(i915, PAGE_SIZE); >> if (IS_ERR(obj)) >> return PTR_ERR(obj); >
Hi Nirmoy, On Fri, Jun 30, 2023 at 07:01:40PM +0200, Nirmoy Das wrote: > Use smem on MTL due to a HW bug in MTL that prevents > reading from stolen memory using LMEM BAR. > > Cc: Oak Zeng <oak.zeng@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Cc: Andrzej Hajda <andrzej.hajda@intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Andi
On 10/07/2023 10:00, Nirmoy Das wrote: > Hi Tvrkto, > > On 7/6/2023 3:43 PM, Tvrtko Ursulin wrote: >> >> On 06/07/2023 14:35, Nirmoy Das wrote: >>> >>> On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote: >>>> >>>> On 30/06/2023 18:01, Nirmoy Das wrote: >>>>> Use smem on MTL due to a HW bug in MTL that prevents >>>>> reading from stolen memory using LMEM BAR. >>>> >>>> Does anything remain in stolen or could the memory region just not >>>> be created? >>> >>> >>> GSC requires DSM which can't use smem for another bug. >> >> Okay, thanks. >> >> As a related comment, these if-if-if object creation ladders were >> always a bit ugly and some years ago I was suggesting we create a >> helper with some "intent/usage" flags. Which could then dtrt ie. >> create the right object for that intent/usage and platform. I *think* >> I possibly even had a RFC... need to try and find it. > > > Did you find it :) Would be nice to have a better way to detect and > apply memory region as per platfrom/usecase. Nope. Basically the idea boiled down to figuring out if it is possible to express the "requirements" via intent flags. Like do we need CPU access, is it mostly GPU, can it be volatile etc. And then combine the intent with the platform to figure out what kind of object to create. But it was many years ago and I am not sure if the idea would still apply so easily, without looking at the all call sites. Could easily end up complicated so I cannot dare to say it is worth spending time looking at this. Regards, Tvrtko > > > Regards, > > Nirmoy > >> >> Regards, >> >> Tvrtko >> >>> >>> Regards, >>> >>> Nirmoy >>> >>> >>>> >>>> Regards, >>>> >>>> Tvrtko >>>> >>>>> Cc: Oak Zeng <oak.zeng@intel.com> >>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >>>>> Cc: Andi Shyti <andi.shyti@linux.intel.com> >>>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >>>>> --- >>>>> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ >>>>> drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- >>>>> 2 files changed, 6 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>> index 1cc0ddc6a310..10e38d60f9ef 100644 >>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>> @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper >>>>> *helper, >>>>> obj = i915_gem_object_create_lmem(dev_priv, size, >>>>> I915_BO_ALLOC_CONTIGUOUS | >>>>> I915_BO_ALLOC_USER); >>>>> + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ >>>>> + obj = i915_gem_object_create_shmem(dev_priv, size); >>>>> } else { >>>>> /* >>>>> * If the FB is too big, just don't use it since fbdev is >>>>> not very >>>>> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c >>>>> b/drivers/gpu/drm/i915/display/intel_overlay.c >>>>> index d6fe2bbabe55..05ae446c8a56 100644 >>>>> --- a/drivers/gpu/drm/i915/display/intel_overlay.c >>>>> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c >>>>> @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct >>>>> drm_device *dev, void *data, >>>>> static int get_registers(struct intel_overlay *overlay, bool >>>>> use_phys) >>>>> { >>>>> struct drm_i915_private *i915 = overlay->i915; >>>>> - struct drm_i915_gem_object *obj; >>>>> + struct drm_i915_gem_object *obj = NULL; >>>>> struct i915_vma *vma; >>>>> int err; >>>>> - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>>>> - if (IS_ERR(obj)) >>>>> + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ >>>>> + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>>>> + if (IS_ERR_OR_NULL(obj)) >>>>> obj = i915_gem_object_create_internal(i915, PAGE_SIZE); >>>>> if (IS_ERR(obj)) >>>>> return PTR_ERR(obj);
On 7/11/2023 5:59 PM, Tvrtko Ursulin wrote: > > On 10/07/2023 10:00, Nirmoy Das wrote: >> Hi Tvrkto, >> >> On 7/6/2023 3:43 PM, Tvrtko Ursulin wrote: >>> >>> On 06/07/2023 14:35, Nirmoy Das wrote: >>>> >>>> On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote: >>>>> >>>>> On 30/06/2023 18:01, Nirmoy Das wrote: >>>>>> Use smem on MTL due to a HW bug in MTL that prevents >>>>>> reading from stolen memory using LMEM BAR. >>>>> >>>>> Does anything remain in stolen or could the memory region just not >>>>> be created? >>>> >>>> >>>> GSC requires DSM which can't use smem for another bug. >>> >>> Okay, thanks. >>> >>> As a related comment, these if-if-if object creation ladders were >>> always a bit ugly and some years ago I was suggesting we create a >>> helper with some "intent/usage" flags. Which could then dtrt ie. >>> create the right object for that intent/usage and platform. I >>> *think* I possibly even had a RFC... need to try and find it. >> >> >> Did you find it :) Would be nice to have a better way to detect and >> apply memory region as per platfrom/usecase. > > Nope. Basically the idea boiled down to figuring out if it is possible > to express the "requirements" via intent flags. Like do we need CPU > access, is it mostly GPU, can it be volatile etc. And then combine the > intent with the platform to figure out what kind of object to create. Yes, that would be nice. > But it was many years ago and I am not sure if the idea would still > apply so easily, without looking at the all call sites. Could easily > end up complicated so I cannot dare to say it is worth spending time > looking at this. I will record this in a Jira and hopefully get back to this when I have less urgent work. Thanks, Nirmoy > > Regards, > > Tvrtko > >> >> >> Regards, >> >> Nirmoy >> >>> >>> Regards, >>> >>> Tvrtko >>> >>>> >>>> Regards, >>>> >>>> Nirmoy >>>> >>>> >>>>> >>>>> Regards, >>>>> >>>>> Tvrtko >>>>> >>>>>> Cc: Oak Zeng <oak.zeng@intel.com> >>>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >>>>>> Cc: Andi Shyti <andi.shyti@linux.intel.com> >>>>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com> >>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ >>>>>> drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- >>>>>> 2 files changed, 6 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>>> index 1cc0ddc6a310..10e38d60f9ef 100644 >>>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >>>>>> @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper >>>>>> *helper, >>>>>> obj = i915_gem_object_create_lmem(dev_priv, size, >>>>>> I915_BO_ALLOC_CONTIGUOUS | >>>>>> I915_BO_ALLOC_USER); >>>>>> + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ >>>>>> + obj = i915_gem_object_create_shmem(dev_priv, size); >>>>>> } else { >>>>>> /* >>>>>> * If the FB is too big, just don't use it since fbdev >>>>>> is not very >>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c >>>>>> b/drivers/gpu/drm/i915/display/intel_overlay.c >>>>>> index d6fe2bbabe55..05ae446c8a56 100644 >>>>>> --- a/drivers/gpu/drm/i915/display/intel_overlay.c >>>>>> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c >>>>>> @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct >>>>>> drm_device *dev, void *data, >>>>>> static int get_registers(struct intel_overlay *overlay, bool >>>>>> use_phys) >>>>>> { >>>>>> struct drm_i915_private *i915 = overlay->i915; >>>>>> - struct drm_i915_gem_object *obj; >>>>>> + struct drm_i915_gem_object *obj = NULL; >>>>>> struct i915_vma *vma; >>>>>> int err; >>>>>> - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>>>>> - if (IS_ERR(obj)) >>>>>> + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ >>>>>> + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); >>>>>> + if (IS_ERR_OR_NULL(obj)) >>>>>> obj = i915_gem_object_create_internal(i915, PAGE_SIZE); >>>>>> if (IS_ERR(obj)) >>>>>> return PTR_ERR(obj);
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index 1cc0ddc6a310..10e38d60f9ef 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper, obj = i915_gem_object_create_lmem(dev_priv, size, I915_BO_ALLOC_CONTIGUOUS | I915_BO_ALLOC_USER); + } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */ + obj = i915_gem_object_create_shmem(dev_priv, size); } else { /* * If the FB is too big, just don't use it since fbdev is not very diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c index d6fe2bbabe55..05ae446c8a56 100644 --- a/drivers/gpu/drm/i915/display/intel_overlay.c +++ b/drivers/gpu/drm/i915/display/intel_overlay.c @@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data, static int get_registers(struct intel_overlay *overlay, bool use_phys) { struct drm_i915_private *i915 = overlay->i915; - struct drm_i915_gem_object *obj; + struct drm_i915_gem_object *obj = NULL; struct i915_vma *vma; int err; - obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); - if (IS_ERR(obj)) + if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */ + obj = i915_gem_object_create_stolen(i915, PAGE_SIZE); + if (IS_ERR_OR_NULL(obj)) obj = i915_gem_object_create_internal(i915, PAGE_SIZE); if (IS_ERR(obj)) return PTR_ERR(obj);
Use smem on MTL due to a HW bug in MTL that prevents reading from stolen memory using LMEM BAR. Cc: Oak Zeng <oak.zeng@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Andi Shyti <andi.shyti@linux.intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> --- drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++ drivers/gpu/drm/i915/display/intel_overlay.c | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-)