[PATCH] drm/i915: Use helper to set CRTC state's mode

[PATCH] drm/i915: Use helper to set CRTC state's mode

[Matt Roper]

We need to call drm_atomic_set_mode_for_crtc() rather than copying the
mode in manually.  As of commit

        commit 99cf4a29fa24461bbfe22125967188a18383eb5c
        Author: Daniel Stone <daniels@collabora.com>
        Date:   Mon May 25 19:11:51 2015 +0100

            drm/atomic: Add current-mode blob to CRTC state

the helper now also takes care of setting up the mode property blob for
us; if we don't use the helper and never setup the mode blob, this will
also trigger a failure in drm_atomic_crtc_check() when we have the
DRIVER_ATOMIC flag set (i.e., when using the nuclear pageflip support
via i915.nuclear_pageflip kernel command line parameter).

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c38c297..4828716 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13192,8 +13192,9 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	if (IS_ERR(crtc_state))
 		return PTR_ERR(crtc_state);
 
-	if (set->mode)
-		drm_mode_copy(&crtc_state->mode, set->mode);
+	ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode);
+	if (ret)
+		return ret;
 
 	if (set->num_connectors)
 		crtc_state->active = true;
-- 
1.8.5.1

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

Re: [PATCH] drm/i915: Use helper to set CRTC state's mode

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6547
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  303/303              303/303
SNB                                  312/312              312/312
IVB                                  343/343              343/343
BYT                                  287/287              287/287
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use helper to set CRTC state's mode

[Maarten Lankhorst]

Hey,

Op 06-06-15 om 00:08 schreef Matt Roper:
> We need to call drm_atomic_set_mode_for_crtc() rather than copying the
> mode in manually.  As of commit
>
>         commit 99cf4a29fa24461bbfe22125967188a18383eb5c
>         Author: Daniel Stone <daniels@collabora.com>
>         Date:   Mon May 25 19:11:51 2015 +0100
>
>             drm/atomic: Add current-mode blob to CRTC state
>
> the helper now also takes care of setting up the mode property blob for
> us; if we don't use the helper and never setup the mode blob, this will
> also trigger a failure in drm_atomic_crtc_check() when we have the
> DRIVER_ATOMIC flag set (i.e., when using the nuclear pageflip support
> via i915.nuclear_pageflip kernel command line parameter).
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
You're probably correct, but convert to atomic part 3 removes this code entirely. :-)

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use helper to set CRTC state's mode

[Matt Roper]

On Mon, Jun 08, 2015 at 06:59:38AM +0200, Maarten Lankhorst wrote:
> Hey,
> 
> Op 06-06-15 om 00:08 schreef Matt Roper:
> > We need to call drm_atomic_set_mode_for_crtc() rather than copying the
> > mode in manually.  As of commit
> >
> >         commit 99cf4a29fa24461bbfe22125967188a18383eb5c
> >         Author: Daniel Stone <daniels@collabora.com>
> >         Date:   Mon May 25 19:11:51 2015 +0100
> >
> >             drm/atomic: Add current-mode blob to CRTC state
> >
> > the helper now also takes care of setting up the mode property blob for
> > us; if we don't use the helper and never setup the mode blob, this will
> > also trigger a failure in drm_atomic_crtc_check() when we have the
> > DRIVER_ATOMIC flag set (i.e., when using the nuclear pageflip support
> > via i915.nuclear_pageflip kernel command line parameter).
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> You're probably correct, but convert to atomic part 3 removes this code entirely. :-)

I'm probably getting confused by all the different spins of atomic
conversion series now, but it looks like this is still needed, even on
top of your latest "Convert to atomic, part 3" set (from June 15th);
we'll end up with no mode blob which leads to warnings and failures from
the DRM core if we don't apply a fix like this to use the helper
function.


Matt

> 
> ~Maarten

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use helper to set CRTC state's mode

[Maarten Lankhorst]

Hey,

Op 17-06-15 om 00:12 schreef Matt Roper:
> On Mon, Jun 08, 2015 at 06:59:38AM +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> Op 06-06-15 om 00:08 schreef Matt Roper:
>>> We need to call drm_atomic_set_mode_for_crtc() rather than copying the
>>> mode in manually.  As of commit
>>>
>>>         commit 99cf4a29fa24461bbfe22125967188a18383eb5c
>>>         Author: Daniel Stone <daniels@collabora.com>
>>>         Date:   Mon May 25 19:11:51 2015 +0100
>>>
>>>             drm/atomic: Add current-mode blob to CRTC state
>>>
>>> the helper now also takes care of setting up the mode property blob for
>>> us; if we don't use the helper and never setup the mode blob, this will
>>> also trigger a failure in drm_atomic_crtc_check() when we have the
>>> DRIVER_ATOMIC flag set (i.e., when using the nuclear pageflip support
>>> via i915.nuclear_pageflip kernel command line parameter).
>>>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> You're probably correct, but convert to atomic part 3 removes this code entirely. :-)
> I'm probably getting confused by all the different spins of atomic
> conversion series now, but it looks like this is still needed, even on
> top of your latest "Convert to atomic, part 3" set (from June 15th);
> we'll end up with no mode blob which leads to warnings and failures from
> the DRM core if we don't apply a fix like this to use the helper
> function.
Oh indeed it's going to take a while for the intel-specific stuff to be removed, feel free to apply and add my r-b.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use helper to set CRTC state's mode

[Daniel Vetter]

On Wed, Jun 17, 2015 at 07:47:23AM +0200, Maarten Lankhorst wrote:
> Hey,
> 
> Op 17-06-15 om 00:12 schreef Matt Roper:
> > On Mon, Jun 08, 2015 at 06:59:38AM +0200, Maarten Lankhorst wrote:
> >> Hey,
> >>
> >> Op 06-06-15 om 00:08 schreef Matt Roper:
> >>> We need to call drm_atomic_set_mode_for_crtc() rather than copying the
> >>> mode in manually.  As of commit
> >>>
> >>>         commit 99cf4a29fa24461bbfe22125967188a18383eb5c
> >>>         Author: Daniel Stone <daniels@collabora.com>
> >>>         Date:   Mon May 25 19:11:51 2015 +0100
> >>>
> >>>             drm/atomic: Add current-mode blob to CRTC state
> >>>
> >>> the helper now also takes care of setting up the mode property blob for
> >>> us; if we don't use the helper and never setup the mode blob, this will
> >>> also trigger a failure in drm_atomic_crtc_check() when we have the
> >>> DRIVER_ATOMIC flag set (i.e., when using the nuclear pageflip support
> >>> via i915.nuclear_pageflip kernel command line parameter).
> >>>
> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> >> You're probably correct, but convert to atomic part 3 removes this code entirely. :-)
> > I'm probably getting confused by all the different spins of atomic
> > conversion series now, but it looks like this is still needed, even on
> > top of your latest "Convert to atomic, part 3" set (from June 15th);
> > we'll end up with no mode blob which leads to warnings and failures from
> > the DRM core if we don't apply a fix like this to use the helper
> > function.
> Oh indeed it's going to take a while for the intel-specific stuff to be removed, feel free to apply and add my r-b.
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx