On 8/26/15, 1:23 AM, Deepak <deepak.s@xxxxxxxxxxxxxxx> wrote:
>
>
>On 08/25/2015 10:18 PM, Chris Wilson wrote:
>> A long time ago (before 3.14) we relied on a permanent pinning of the
>> ifbdev to lock the fb in place inside the GGTT. However, the
>> introduction of stealing the BIOS framebuffer and reusing its address in
>> the GGTT for the fbdev has muddied waters and we use an inherited fb.
>> However, the inherited fb is only pinned whilst it is active and we no
>> longer have an explicit pin for the info->system_base mmapping used by
>> the fbdev. The result is that after some aperture pressure the fbdev may
>> be evicted, but we continue to write the fbcon into the same GGTT
>> address - overwriting anything else that may be put into that offset.
>> The effect is most pronounced across suspend/resume as
>> intel_fbdev_set_suspend() does a full clear over the whole scanout.
>
>Yup this is a critical fix :) by keeping the internal FB pinned we avoid
>alloc of buffer within same FB GTT offset
>Reviewed-by: Deepak S <deepak.s@xxxxxxxxxxxxxxx>
Daniel,
Is there anything else that needs to happen before this gets pulled in?
Thanks,
Wayne
>
>> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> Cc: "Goel, Akash" <akash.goel@xxxxxxxxx>
>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
>> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>> ---
>> drivers/gpu/drm/i915/intel_fbdev.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
>>b/drivers/gpu/drm/i915/intel_fbdev.c
>> index 96476d7d7ed2..082f2938ec97 100644
>> --- a/drivers/gpu/drm/i915/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
>> @@ -215,6 +215,16 @@ static int intelfb_create(struct drm_fb_helper
>>*helper,
>> obj = intel_fb->obj;
>> size = obj->base.size;
>>
>> + /* The fb constructor will have already pinned us (or inherited a
>> + * GGTT region from the BIOS) suitable for a scanout, so
>> + * this should just be a no-op and increment the pin count for the
>> + * fbdev mmapping. It does have a useful side-effect of validating
>> + * the pin for fbdev's use via a GGTT mmapping.
>> + */
>> + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, PIN_MAPPABLE);
>> + if (ret)
>> + goto out_unlock;
>> +
>> info = drm_fb_helper_alloc_fbi(helper);
>> if (IS_ERR(info)) {
>> ret = PTR_ERR(info);
>> @@ -274,6 +284,9 @@ static int intelfb_create(struct drm_fb_helper
>>*helper,
>> out_destroy_fbi:
>> drm_fb_helper_release_fbi(helper);
>> out_unpin:
>> + /* Once for info->screen_base mmaping... */
>> + i915_gem_object_ggtt_unpin(obj);
>> + /* ...and once for the intel_fb */
>> i915_gem_object_ggtt_unpin(obj);
>> drm_gem_object_unreference(&obj->base);
>> out_unlock:
>> @@ -514,6 +527,8 @@ static const struct drm_fb_helper_funcs
>>intel_fb_helper_funcs = {
>> static void intel_fbdev_destroy(struct drm_device *dev,
>> struct intel_fbdev *ifbdev)
>> {
>> + /* Release the pinning for the info->screen_base mmaping. */
>> + i915_gem_object_ggtt_unpin(ifbdev->fb->obj);
>>
>> drm_fb_helper_unregister_fbi(&ifbdev->helper);
>> drm_fb_helper_release_fbi(&ifbdev->helper);
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
