Re: [PATCH v3 06/20] drm/i915: Remove plane_config from struct intel_crtc.

From: Daniel Stone <daniel@fooishbar.org>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v3 06/20] drm/i915: Remove plane_config from struct intel_crtc.
Date: Tue, 14 Jul 2015 12:23:50 +0100	[thread overview]
Message-ID: <CAPj87rOM+EuXcgY=jvFDiHCa=tVoTiBGp6OC+7BK9C5+3OeHKQ@mail.gmail.com> (raw)
In-Reply-To: <55A4C80B.2010104@linux.intel.com>

Hey,

On 14 July 2015 at 09:27, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Op 13-07-15 om 18:30 schreef Daniel Stone:
>> On 13 July 2015 at 15:30, Maarten Lankhorst
>> <maarten.lankhorst@linux.intel.com> wrote:
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 037a85f1b127..e4d8acc63823 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -15204,7 +15204,9 @@ void intel_modeset_init(struct drm_device *dev)
>>>         drm_modeset_unlock_all(dev);
>>>
>>>         for_each_intel_crtc(dev, crtc) {
>>> -               if (!crtc->active)
>>> +               struct intel_initial_plane_config plane_config;
>>> +
>>> +               if (!crtc->base.state->active)
>> Unrelated change from crtc->active to crtc->base.state->active - can
>> we do this in one of the later patches?
> Probably, but I'm trying to get rid of crtc->active every time I touch a function.

Sure, but it does make bisection a bit more difficult.

> Eventually this will mean being able to remove it. ;-)

Hey, you know I'm in favour of this!

>>> +               plane_config.fb = NULL;
>> memset to 0 instead?
> Well for intel_find_initial_plane_obj it's sufficient, but I can just initialize with plane_config = {}; to keep old behavior.
>>> @@ -15713,6 +15716,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
>>>                 if (ret) {
>>>                         DRM_ERROR("failed to pin boot fb on pipe %d\n",
>>>                                   to_intel_crtc(c)->pipe);
>>> +                       obj->frontbuffer_bits &=
>>> +                               ~to_intel_plane(c->primary)->frontbuffer_bit;
>>>                         drm_framebuffer_unreference(c->primary->fb);
>>>                         c->primary->fb = NULL;
>>>                         c->primary->crtc = c->primary->state->crtc = NULL;
>> Unrelated change?
> Unrelated perhaps, but it fixes a warn when pinning fails.
> Still I guess a WARN won't hurt in that case, I'll drop it.

Yeah, it does make sense to me, but then again it wouldn't be the
first time that a frontbuffer-tracking change I've thought made sense,
actually broke things. ;)

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx