[PATCH v4 0/9] drm/i915: Convert connector properties to atomic.

[PATCH v4 0/9] drm/i915: Convert connector properties to atomic.

[Maarten Lankhorst]

First part has been reviewed and upstreamed, second part resubmitted
here with fix to core to handle aspect ratio and scaling mode. :)

Only 'add plumbing for digital connector state' had significant changes,
rest was just fixed to use the new place for aspect ratio and scaling mode.

Maarten Lankhorst (9):
  drm/atomic: Handle aspect ratio and scaling mode in core
  drm/i915: Add plumbing for digital connector state, v2.
  drm/i915: Convert DSI connector properties to atomic.
  drm/i915: Convert LVDS connector properties to atomic.
  drm/i915: Make intel_dp->has_audio reflect hw state only
  drm/i915: Convert intel_dp properties to atomic.
  drm/i915: Convert intel_hdmi connector properties to atomic
  drm/i915: Handle force_audio correctly in intel_sdvo
  drm/i915: Convert intel_sdvo connector properties to atomic.

 drivers/gpu/drm/drm_atomic.c         |   8 +
 drivers/gpu/drm/i915/intel_atomic.c  | 146 +++++++--
 drivers/gpu/drm/i915/intel_display.c |  51 +---
 drivers/gpu/drm/i915/intel_dp.c      | 152 ++--------
 drivers/gpu/drm/i915/intel_drv.h     |  38 ++-
 drivers/gpu/drm/i915/intel_dsi.c     |  59 +---
 drivers/gpu/drm/i915/intel_dvo.c     |   2 +-
 drivers/gpu/drm/i915/intel_hdmi.c    | 164 +++-------
 drivers/gpu/drm/i915/intel_lvds.c    |  56 +---
 drivers/gpu/drm/i915/intel_panel.c   |   4 +-
 drivers/gpu/drm/i915/intel_sdvo.c    | 561 ++++++++++++++++++-----------------
 include/drm/drm_connector.h          |   3 +
 12 files changed, 542 insertions(+), 702 deletions(-)

-- 
2.7.4

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

[PATCH v4 1/9] drm/atomic: Handle aspect ratio and scaling mode in core

[Maarten Lankhorst]

This is required to for i915 to convert connector properties to atomic.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_atomic.c | 8 ++++++++
 include/drm/drm_connector.h  | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f32506a7c1d6..052cfe2376f8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
 		 */
 		if (state->link_status != DRM_LINK_STATUS_GOOD)
 			state->link_status = val;
+	} else if (property == config->aspect_ratio_property) {
+		state->picture_aspect_ratio = val;
+	} else if (property == config->scaling_mode_property) {
+		state->scaling_mode = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->tv.hue;
 	} else if (property == config->link_status_property) {
 		*val = state->link_status;
+	} else if (property == config->aspect_ratio_property) {
+		*val = state->picture_aspect_ratio;
+	} else if (property == config->scaling_mode_property) {
+		*val = state->scaling_mode;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4eeda120e46d..3b819cd1d858 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -326,6 +326,9 @@ struct drm_connector_state {
 	struct drm_atomic_state *state;
 
 	struct drm_tv_connector_state tv;
+
+	unsigned int picture_aspect_ratio;
+	unsigned int scaling_mode;
 };
 
 /**
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4.1 01/9] drm/atomic: Handle aspect ratio and scaling mode in core, v2.

[Maarten Lankhorst]

This is required to for i915 to convert connector properties to atomic.

Changes since v1:
- Add docbook info. (danvet)
- Change picture_aspect_ratio to enum hdmi_picture_aspect.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Acked-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_atomic.c |  8 ++++++++
 include/drm/drm_connector.h  | 16 ++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 30229ab719c0..25ea6f797a54 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
 		 */
 		if (state->link_status != DRM_LINK_STATUS_GOOD)
 			state->link_status = val;
+	} else if (property == config->aspect_ratio_property) {
+		state->picture_aspect_ratio = val;
+	} else if (property == config->scaling_mode_property) {
+		state->scaling_mode = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->tv.hue;
 	} else if (property == config->link_status_property) {
 		*val = state->link_status;
+	} else if (property == config->aspect_ratio_property) {
+		*val = state->picture_aspect_ratio;
+	} else if (property == config->scaling_mode_property) {
+		*val = state->scaling_mode;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4eeda120e46d..5b50bc2db6fb 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -25,6 +25,7 @@
 
 #include <linux/list.h>
 #include <linux/ctype.h>
+#include <linux/hdmi.h>
 #include <drm/drm_mode_object.h>
 
 #include <uapi/drm/drm_mode.h>
@@ -326,6 +327,21 @@ struct drm_connector_state {
 	struct drm_atomic_state *state;
 
 	struct drm_tv_connector_state tv;
+
+	/**
+	 * @picture_aspect_ratio: Connector property to control the
+	 * HDMI infoframe aspect ratio setting.
+	 *
+	 * The DRM_MODE_PICTURE_ASPECT_* values much match the
+	 * values for &enum hdmi_picture_aspect
+	 */
+	enum hdmi_picture_aspect picture_aspect_ratio;
+
+	/**
+	 * @scaling_mode: Connector property to control the
+	 * upscaling, mostly used for built-in panels.
+	 */
+	unsigned int scaling_mode;
 };
 
 /**
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4.1 01/9] drm/atomic: Handle aspect ratio and scaling mode in core, v2.

[Daniel Vetter]

On Thu, Apr 13, 2017 at 11:15:37AM +0200, Maarten Lankhorst wrote:
> This is required to for i915 to convert connector properties to atomic.
> 
> Changes since v1:
> - Add docbook info. (danvet)
> - Change picture_aspect_ratio to enum hdmi_picture_aspect.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Acked-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |  8 ++++++++
>  include/drm/drm_connector.h  | 16 ++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 30229ab719c0..25ea6f797a54 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		 */
>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
>  			state->link_status = val;
> +	} else if (property == config->aspect_ratio_property) {
> +		state->picture_aspect_ratio = val;
> +	} else if (property == config->scaling_mode_property) {
> +		state->scaling_mode = val;
>  	} else if (connector->funcs->atomic_set_property) {
>  		return connector->funcs->atomic_set_property(connector,
>  				state, property, val);
> @@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = state->tv.hue;
>  	} else if (property == config->link_status_property) {
>  		*val = state->link_status;
> +	} else if (property == config->aspect_ratio_property) {
> +		*val = state->picture_aspect_ratio;
> +	} else if (property == config->scaling_mode_property) {
> +		*val = state->scaling_mode;
>  	} else if (connector->funcs->atomic_get_property) {
>  		return connector->funcs->atomic_get_property(connector,
>  				state, property, val);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4eeda120e46d..5b50bc2db6fb 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -25,6 +25,7 @@
>  
>  #include <linux/list.h>
>  #include <linux/ctype.h>
> +#include <linux/hdmi.h>
>  #include <drm/drm_mode_object.h>
>  
>  #include <uapi/drm/drm_mode.h>
> @@ -326,6 +327,21 @@ struct drm_connector_state {
>  	struct drm_atomic_state *state;
>  
>  	struct drm_tv_connector_state tv;
> +
> +	/**
> +	 * @picture_aspect_ratio: Connector property to control the
> +	 * HDMI infoframe aspect ratio setting.
> +	 *
> +	 * The DRM_MODE_PICTURE_ASPECT_* values much match the

I think the above will upset sphinx and spew a warning, you need
ASPECT_\* or something like that. Or just spell them out and enumerate
them all. Fixed either way this is

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +	 * values for &enum hdmi_picture_aspect
> +	 */
> +	enum hdmi_picture_aspect picture_aspect_ratio;
> +
> +	/**
> +	 * @scaling_mode: Connector property to control the
> +	 * upscaling, mostly used for built-in panels.
> +	 */
> +	unsigned int scaling_mode;
>  };
>  
>  /**
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4.1 01/9] drm/atomic: Handle aspect ratio and scaling mode in core, v2.

[Maarten Lankhorst]

On 19-04-17 17:43, Daniel Vetter wrote:
> On Thu, Apr 13, 2017 at 11:15:37AM +0200, Maarten Lankhorst wrote:
>> This is required to for i915 to convert connector properties to atomic.
>>
>> Changes since v1:
>> - Add docbook info. (danvet)
>> - Change picture_aspect_ratio to enum hdmi_picture_aspect.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Acked-by: Dave Airlie <airlied@redhat.com>
>> ---
>>  drivers/gpu/drm/drm_atomic.c |  8 ++++++++
>>  include/drm/drm_connector.h  | 16 ++++++++++++++++
>>  2 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 30229ab719c0..25ea6f797a54 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>>  		 */
>>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
>>  			state->link_status = val;
>> +	} else if (property == config->aspect_ratio_property) {
>> +		state->picture_aspect_ratio = val;
>> +	} else if (property == config->scaling_mode_property) {
>> +		state->scaling_mode = val;
>>  	} else if (connector->funcs->atomic_set_property) {
>>  		return connector->funcs->atomic_set_property(connector,
>>  				state, property, val);
>> @@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>  		*val = state->tv.hue;
>>  	} else if (property == config->link_status_property) {
>>  		*val = state->link_status;
>> +	} else if (property == config->aspect_ratio_property) {
>> +		*val = state->picture_aspect_ratio;
>> +	} else if (property == config->scaling_mode_property) {
>> +		*val = state->scaling_mode;
>>  	} else if (connector->funcs->atomic_get_property) {
>>  		return connector->funcs->atomic_get_property(connector,
>>  				state, property, val);
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 4eeda120e46d..5b50bc2db6fb 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -25,6 +25,7 @@
>>  
>>  #include <linux/list.h>
>>  #include <linux/ctype.h>
>> +#include <linux/hdmi.h>
>>  #include <drm/drm_mode_object.h>
>>  
>>  #include <uapi/drm/drm_mode.h>
>> @@ -326,6 +327,21 @@ struct drm_connector_state {
>>  	struct drm_atomic_state *state;
>>  
>>  	struct drm_tv_connector_state tv;
>> +
>> +	/**
>> +	 * @picture_aspect_ratio: Connector property to control the
>> +	 * HDMI infoframe aspect ratio setting.
>> +	 *
>> +	 * The DRM_MODE_PICTURE_ASPECT_* values much match the
> I think the above will upset sphinx and spew a warning, you need
> ASPECT_\* or something like that. Or just spell them out and enumerate
> them all. Fixed either way this is
I checked and make htmldocs worked just fine, but I'll quote the *.

Thanks,
Maarten
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4.1 01/9] drm/atomic: Handle aspect ratio and scaling mode in core, v2.

[Daniel Vetter]

On Mon, Apr 24, 2017 at 02:01:05PM +0200, Maarten Lankhorst wrote:
> On 19-04-17 17:43, Daniel Vetter wrote:
> > On Thu, Apr 13, 2017 at 11:15:37AM +0200, Maarten Lankhorst wrote:
> >> This is required to for i915 to convert connector properties to atomic.
> >>
> >> Changes since v1:
> >> - Add docbook info. (danvet)
> >> - Change picture_aspect_ratio to enum hdmi_picture_aspect.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: dri-devel@lists.freedesktop.org
> >> Acked-by: Dave Airlie <airlied@redhat.com>
> >> ---
> >>  drivers/gpu/drm/drm_atomic.c |  8 ++++++++
> >>  include/drm/drm_connector.h  | 16 ++++++++++++++++
> >>  2 files changed, 24 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >> index 30229ab719c0..25ea6f797a54 100644
> >> --- a/drivers/gpu/drm/drm_atomic.c
> >> +++ b/drivers/gpu/drm/drm_atomic.c
> >> @@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
> >>  		 */
> >>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
> >>  			state->link_status = val;
> >> +	} else if (property == config->aspect_ratio_property) {
> >> +		state->picture_aspect_ratio = val;
> >> +	} else if (property == config->scaling_mode_property) {
> >> +		state->scaling_mode = val;
> >>  	} else if (connector->funcs->atomic_set_property) {
> >>  		return connector->funcs->atomic_set_property(connector,
> >>  				state, property, val);
> >> @@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >>  		*val = state->tv.hue;
> >>  	} else if (property == config->link_status_property) {
> >>  		*val = state->link_status;
> >> +	} else if (property == config->aspect_ratio_property) {
> >> +		*val = state->picture_aspect_ratio;
> >> +	} else if (property == config->scaling_mode_property) {
> >> +		*val = state->scaling_mode;
> >>  	} else if (connector->funcs->atomic_get_property) {
> >>  		return connector->funcs->atomic_get_property(connector,
> >>  				state, property, val);
> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> >> index 4eeda120e46d..5b50bc2db6fb 100644
> >> --- a/include/drm/drm_connector.h
> >> +++ b/include/drm/drm_connector.h
> >> @@ -25,6 +25,7 @@
> >>  
> >>  #include <linux/list.h>
> >>  #include <linux/ctype.h>
> >> +#include <linux/hdmi.h>
> >>  #include <drm/drm_mode_object.h>
> >>  
> >>  #include <uapi/drm/drm_mode.h>
> >> @@ -326,6 +327,21 @@ struct drm_connector_state {
> >>  	struct drm_atomic_state *state;
> >>  
> >>  	struct drm_tv_connector_state tv;
> >> +
> >> +	/**
> >> +	 * @picture_aspect_ratio: Connector property to control the
> >> +	 * HDMI infoframe aspect ratio setting.
> >> +	 *
> >> +	 * The DRM_MODE_PICTURE_ASPECT_* values much match the
> > I think the above will upset sphinx and spew a warning, you need
> > ASPECT_\* or something like that. Or just spell them out and enumerate
> > them all. Fixed either way this is
> I checked and make htmldocs worked just fine, but I'll quote the *.

I meant inspecting the visual output ... Without the * escaped iirc you
can end up with the entire paragraph bold.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4.1 01/9] drm/atomic: Handle aspect ratio and scaling mode in core, v2.

[Maarten Lankhorst]

Op 02-05-17 om 10:12 schreef Daniel Vetter:
> On Mon, Apr 24, 2017 at 02:01:05PM +0200, Maarten Lankhorst wrote:
>> On 19-04-17 17:43, Daniel Vetter wrote:
>>> On Thu, Apr 13, 2017 at 11:15:37AM +0200, Maarten Lankhorst wrote:
>>>> This is required to for i915 to convert connector properties to atomic.
>>>>
>>>> Changes since v1:
>>>> - Add docbook info. (danvet)
>>>> - Change picture_aspect_ratio to enum hdmi_picture_aspect.
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Cc: dri-devel@lists.freedesktop.org
>>>> Acked-by: Dave Airlie <airlied@redhat.com>
>>>> ---
>>>>  drivers/gpu/drm/drm_atomic.c |  8 ++++++++
>>>>  include/drm/drm_connector.h  | 16 ++++++++++++++++
>>>>  2 files changed, 24 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>>> index 30229ab719c0..25ea6f797a54 100644
>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>> @@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>>>>  		 */
>>>>  		if (state->link_status != DRM_LINK_STATUS_GOOD)
>>>>  			state->link_status = val;
>>>> +	} else if (property == config->aspect_ratio_property) {
>>>> +		state->picture_aspect_ratio = val;
>>>> +	} else if (property == config->scaling_mode_property) {
>>>> +		state->scaling_mode = val;
>>>>  	} else if (connector->funcs->atomic_set_property) {
>>>>  		return connector->funcs->atomic_set_property(connector,
>>>>  				state, property, val);
>>>> @@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>>>  		*val = state->tv.hue;
>>>>  	} else if (property == config->link_status_property) {
>>>>  		*val = state->link_status;
>>>> +	} else if (property == config->aspect_ratio_property) {
>>>> +		*val = state->picture_aspect_ratio;
>>>> +	} else if (property == config->scaling_mode_property) {
>>>> +		*val = state->scaling_mode;
>>>>  	} else if (connector->funcs->atomic_get_property) {
>>>>  		return connector->funcs->atomic_get_property(connector,
>>>>  				state, property, val);
>>>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>>>> index 4eeda120e46d..5b50bc2db6fb 100644
>>>> --- a/include/drm/drm_connector.h
>>>> +++ b/include/drm/drm_connector.h
>>>> @@ -25,6 +25,7 @@
>>>>  
>>>>  #include <linux/list.h>
>>>>  #include <linux/ctype.h>
>>>> +#include <linux/hdmi.h>
>>>>  #include <drm/drm_mode_object.h>
>>>>  
>>>>  #include <uapi/drm/drm_mode.h>
>>>> @@ -326,6 +327,21 @@ struct drm_connector_state {
>>>>  	struct drm_atomic_state *state;
>>>>  
>>>>  	struct drm_tv_connector_state tv;
>>>> +
>>>> +	/**
>>>> +	 * @picture_aspect_ratio: Connector property to control the
>>>> +	 * HDMI infoframe aspect ratio setting.
>>>> +	 *
>>>> +	 * The DRM_MODE_PICTURE_ASPECT_* values much match the
>>> I think the above will upset sphinx and spew a warning, you need
>>> ASPECT_\* or something like that. Or just spell them out and enumerate
>>> them all. Fixed either way this is
>> I checked and make htmldocs worked just fine, but I'll quote the *.
> I meant inspecting the visual output ... Without the * escaped iirc you
> can end up with the entire paragraph bold.
> -Daniel

Yeah either way worked though for the html documentation, at least when I checked it locally. But it didn't complain either option so v5 and v6 use \*.


~Maarten

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 2/9] drm/i915: Add plumbing for digital connector state, v2.

[Maarten Lankhorst]

Some atomic properties are common between the various kinds of
connectors, for example a lot of them use panel fitting mode.
It makes sense to put a lot of it in a common place, so each
connector can use it while they're being converted.

Implement the properties required for the connectors:
- scaling mode property
- force audio property
- broadcast rgb
- aspect ratio

While at it, make clear that intel_digital_connector_atomic_get_property
is a hack that has to be removed when all connector properties
are converted to atomic.

Changes since v1:
- Scaling mode and aspect ratio are partly handled in core now.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c  | 142 +++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_display.c |  14 +++-
 drivers/gpu/drm/i915/intel_dp.c      |   2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  27 ++++++-
 drivers/gpu/drm/i915/intel_dsi.c     |   2 +-
 drivers/gpu/drm/i915/intel_dvo.c     |   2 +-
 drivers/gpu/drm/i915/intel_lvds.c    |   2 +-
 drivers/gpu/drm/i915/intel_panel.c   |   4 +-
 8 files changed, 180 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 50fb1f76cc5f..eb72166675f0 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -36,7 +36,7 @@
 #include "intel_drv.h"
 
 /**
- * intel_connector_atomic_get_property - fetch connector property value
+ * intel_connector_atomic_get_property - fetch legacy connector property value
  * @connector: connector to fetch property for
  * @state: state containing the property value
  * @property: property to look up
@@ -45,12 +45,14 @@
  * The DRM core does not store shadow copies of properties for
  * atomic-capable drivers.  This entrypoint is used to fetch
  * the current value of a driver-specific connector property.
+ *
+ * This is a intermediary solution until all connectors are
+ * converted to support full atomic properties.
  */
-int
-intel_connector_atomic_get_property(struct drm_connector *connector,
-				    const struct drm_connector_state *state,
-				    struct drm_property *property,
-				    uint64_t *val)
+int intel_connector_atomic_get_property(struct drm_connector *connector,
+					const struct drm_connector_state *state,
+					struct drm_property *property,
+					uint64_t *val)
 {
 	int i;
 
@@ -73,7 +75,133 @@ intel_connector_atomic_get_property(struct drm_connector *connector,
 	return -EINVAL;
 }
 
-/*
+/**
+ * intel_digital_connector_atomic_get_property - hook for connector->atomic_get_property.
+ * @connector: Connector to get the property for.
+ * @state: Connector state to retrieve the property from.
+ * @property: Property to retrieve.
+ * @val: Return value for the property.
+ *
+ * Returns the atomic property value for a digital connector.
+ */
+int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
+						const struct drm_connector_state *state,
+						struct drm_property *property,
+						uint64_t *val)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_digital_connector_state *intel_conn_state =
+		to_intel_digital_connector_state(state);
+
+	if (property == dev_priv->force_audio_property)
+		*val = intel_conn_state->force_audio;
+	else if (property == dev_priv->broadcast_rgb_property)
+		*val = intel_conn_state->broadcast_rgb;
+	else {
+		DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * intel_digital_connector_atomic_set_property - hook for connector->atomic_set_property.
+ * @connector: Connector to set the property for.
+ * @state: Connector state to set the property on.
+ * @property: Property to set.
+ * @val: New value for the property.
+ *
+ * Sets the atomic property value for a digital connector.
+ */
+int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
+						struct drm_connector_state *state,
+						struct drm_property *property,
+						uint64_t val)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_digital_connector_state *intel_conn_state =
+		to_intel_digital_connector_state(state);
+
+	if (property == dev_priv->force_audio_property) {
+		intel_conn_state->force_audio = val;
+		return 0;
+	}
+
+	if (property == dev_priv->broadcast_rgb_property) {
+		intel_conn_state->broadcast_rgb = val;
+		return 0;
+	}
+
+	DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
+	return -EINVAL;
+}
+
+int intel_digital_connector_atomic_check(struct drm_connector *conn,
+					 struct drm_connector_state *new_state)
+{
+	struct intel_digital_connector_state *new_conn_state =
+		to_intel_digital_connector_state(new_state);
+	struct drm_connector_state *old_state =
+		drm_atomic_get_old_connector_state(new_state->state, conn);
+	struct intel_digital_connector_state *old_conn_state =
+		to_intel_digital_connector_state(old_state);
+	struct intel_connector *intel_connector = to_intel_connector(conn);
+	struct drm_crtc_state *crtc_state;
+
+	if (intel_connector->panel.allowed_fitting_mode_mask &&
+	    (BIT(new_state->scaling_mode) & ~intel_connector->panel.allowed_fitting_mode_mask)) {
+		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] Panel fitting mode %u not allowed.\n",
+				 conn->base.id, conn->name, new_state->scaling_mode);
+		return -EINVAL;
+	}
+
+	if (!new_state->crtc)
+		return 0;
+
+	crtc_state = drm_atomic_get_new_crtc_state(new_state->state, new_state->crtc);
+
+	/* Invisible to fastset since it's a pure connector property */
+	if (new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
+		crtc_state->connectors_changed = true;
+
+	/*
+	 * These properties are handled by fastset, and might not end
+	 * up in a modeset.
+	 */
+	if (new_conn_state->force_audio != old_conn_state->force_audio ||
+	    new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
+	    new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio)
+		crtc_state->mode_changed = true;
+
+	return 0;
+}
+
+/**
+ * intel_digital_connector_duplicate_state - duplicate connector state
+ * @connector: digital connector
+ *
+ * Allocates and returns a copy of the connector state (both common and
+ * digital connector specific) for the specified connector.
+ *
+ * Returns: The newly allocated connector state, or NULL on failure.
+ */
+struct drm_connector_state *
+intel_digital_connector_duplicate_state(struct drm_connector *connector)
+{
+	struct intel_digital_connector_state *state;
+
+	state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	__drm_atomic_helper_connector_duplicate_state(connector, &state->base);
+	return &state->base;
+}
+
+/**
  * intel_crtc_duplicate_state - duplicate crtc state
  * @crtc: drm crtc
  *
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 48a546210d8b..5eb4ddb04797 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5956,11 +5956,21 @@ static void intel_connector_verify_state(struct intel_connector *connector)
 
 int intel_connector_init(struct intel_connector *connector)
 {
-	drm_atomic_helper_connector_reset(&connector->base);
+	struct intel_digital_connector_state *conn_state;
 
-	if (!connector->base.state)
+	/*
+	 * Allocate enough memory to hold intel_digital_connector_state,
+	 * This might be a few bytes too many, but for connectors that don't
+	 * need it we'll free the state and allocate a smaller one on the first
+	 * succesful commit anyway.
+	 */
+	conn_state = kzalloc(sizeof(*conn_state), GFP_KERNEL);
+	if (!conn_state)
 		return -ENOMEM;
 
+	__drm_atomic_helper_connector_reset(&connector->base,
+					    &conn_state->base);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 944b1449b7f9..0afd8f3037f0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5896,7 +5896,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 			      pipe_name(pipe));
 	}
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
 	intel_connector->panel.backlight.power = intel_edp_backlight_power;
 	intel_panel_setup_backlight(connector, pipe);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f78d7c5f3805..4df9e4531edc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -262,6 +262,7 @@ struct intel_panel {
 	struct drm_display_mode *fixed_mode;
 	struct drm_display_mode *downclock_mode;
 	int fitting_mode;
+	unsigned allowed_fitting_mode_mask;
 
 	/* backlight */
 	struct {
@@ -323,6 +324,15 @@ struct intel_connector {
 	struct intel_dp *mst_port;
 };
 
+struct intel_digital_connector_state {
+	struct drm_connector_state base;
+
+	enum hdmi_force_audio force_audio;
+	int broadcast_rgb;
+};
+
+#define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base)
+
 struct dpll {
 	/* given values */
 	int n;
@@ -1668,7 +1678,8 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
 /* intel_panel.c */
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode,
-		     struct drm_display_mode *downclock_mode);
+		     struct drm_display_mode *downclock_mode,
+		     unsigned allowed_fitting_mode_mask);
 void intel_panel_fini(struct intel_panel *panel);
 void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 			    struct drm_display_mode *adjusted_mode);
@@ -1879,6 +1890,20 @@ int intel_connector_atomic_get_property(struct drm_connector *connector,
 					const struct drm_connector_state *state,
 					struct drm_property *property,
 					uint64_t *val);
+
+int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
+						const struct drm_connector_state *state,
+						struct drm_property *property,
+						uint64_t *val);
+int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
+						struct drm_connector_state *state,
+						struct drm_property *property,
+						uint64_t val);
+int intel_digital_connector_atomic_check(struct drm_connector *conn,
+					 struct drm_connector_state *new_state);
+struct drm_connector_state *
+intel_digital_connector_duplicate_state(struct drm_connector *connector);
+
 struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
 void intel_crtc_destroy_state(struct drm_crtc *crtc,
 			       struct drm_crtc_state *state);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 3ffe8b1f1d48..e787142025ac 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1853,7 +1853,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	connector->display_info.width_mm = fixed_mode->width_mm;
 	connector->display_info.height_mm = fixed_mode->height_mm;
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
+	intel_panel_init(&intel_connector->panel, fixed_mode, NULL, 0);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	intel_dsi_add_properties(intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index c1544a53095d..0b723157c38b 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -554,7 +554,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
 			 */
 			intel_panel_init(&intel_connector->panel,
 					 intel_dvo_get_current_mode(connector),
-					 NULL);
+					 NULL, 0);
 			intel_dvo->panel_wants_dither = true;
 		}
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 8b942ef2b3ec..919d84290774 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1181,7 +1181,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 out:
 	mutex_unlock(&dev->mode_config.mutex);
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb50c527401f..c6ce15550e5e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1801,12 +1801,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode,
-		     struct drm_display_mode *downclock_mode)
+		     struct drm_display_mode *downclock_mode,
+		     unsigned allowed_fitting_mode_mask)
 {
 	intel_panel_init_backlight_funcs(panel);
 
 	panel->fixed_mode = fixed_mode;
 	panel->downclock_mode = downclock_mode;
+	panel->allowed_fitting_mode_mask = allowed_fitting_mode_mask;
 
 	return 0;
 }
-- 
2.7.4

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

Re: [PATCH v4 2/9] drm/i915: Add plumbing for digital connector state, v2.

[Daniel Vetter]

On Wed, Apr 12, 2017 at 12:50:00PM +0200, Maarten Lankhorst wrote:
> Some atomic properties are common between the various kinds of
> connectors, for example a lot of them use panel fitting mode.
> It makes sense to put a lot of it in a common place, so each
> connector can use it while they're being converted.
> 
> Implement the properties required for the connectors:
> - scaling mode property
> - force audio property
> - broadcast rgb
> - aspect ratio
> 
> While at it, make clear that intel_digital_connector_atomic_get_property
> is a hack that has to be removed when all connector properties
> are converted to atomic.
> 
> Changes since v1:
> - Scaling mode and aspect ratio are partly handled in core now.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  | 142 +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_display.c |  14 +++-
>  drivers/gpu/drm/i915/intel_dp.c      |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  27 ++++++-
>  drivers/gpu/drm/i915/intel_dsi.c     |   2 +-
>  drivers/gpu/drm/i915/intel_dvo.c     |   2 +-
>  drivers/gpu/drm/i915/intel_lvds.c    |   2 +-
>  drivers/gpu/drm/i915/intel_panel.c   |   4 +-
>  8 files changed, 180 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 50fb1f76cc5f..eb72166675f0 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c

Refactoring idea for the future: intel_atomic.c makes 0 sense, just makes
sure we have lots of functionality spread all over imo. I think following
the split-up model I've done in the drm core would be a lot better, with
intel_connector/encoder/crtc/plane.c for the shared helper functions. And
then maybe separate files for the per-platform stuff, e.g. i9xx_crtc.c,
pch_crtc.c and so on.

> @@ -36,7 +36,7 @@
>  #include "intel_drv.h"
>  
>  /**
> - * intel_connector_atomic_get_property - fetch connector property value
> + * intel_connector_atomic_get_property - fetch legacy connector property value
>   * @connector: connector to fetch property for
>   * @state: state containing the property value
>   * @property: property to look up
> @@ -45,12 +45,14 @@
>   * The DRM core does not store shadow copies of properties for
>   * atomic-capable drivers.  This entrypoint is used to fetch
>   * the current value of a driver-specific connector property.
> + *
> + * This is a intermediary solution until all connectors are
> + * converted to support full atomic properties.
>   */
> -int
> -intel_connector_atomic_get_property(struct drm_connector *connector,
> -				    const struct drm_connector_state *state,
> -				    struct drm_property *property,
> -				    uint64_t *val)
> +int intel_connector_atomic_get_property(struct drm_connector *connector,
> +					const struct drm_connector_state *state,
> +					struct drm_property *property,
> +					uint64_t *val)
>  {
>  	int i;
>  
> @@ -73,7 +75,133 @@ intel_connector_atomic_get_property(struct drm_connector *connector,
>  	return -EINVAL;
>  }
>  
> -/*
> +/**
> + * intel_digital_connector_atomic_get_property - hook for connector->atomic_get_property.
> + * @connector: Connector to get the property for.
> + * @state: Connector state to retrieve the property from.
> + * @property: Property to retrieve.
> + * @val: Return value for the property.
> + *
> + * Returns the atomic property value for a digital connector.
> + */
> +int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
> +						const struct drm_connector_state *state,
> +						struct drm_property *property,
> +						uint64_t *val)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_digital_connector_state *intel_conn_state =
> +		to_intel_digital_connector_state(state);
> +
> +	if (property == dev_priv->force_audio_property)
> +		*val = intel_conn_state->force_audio;
> +	else if (property == dev_priv->broadcast_rgb_property)
> +		*val = intel_conn_state->broadcast_rgb;
> +	else {
> +		DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * intel_digital_connector_atomic_set_property - hook for connector->atomic_set_property.
> + * @connector: Connector to set the property for.
> + * @state: Connector state to set the property on.
> + * @property: Property to set.
> + * @val: New value for the property.
> + *
> + * Sets the atomic property value for a digital connector.
> + */
> +int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
> +						struct drm_connector_state *state,
> +						struct drm_property *property,
> +						uint64_t val)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_digital_connector_state *intel_conn_state =
> +		to_intel_digital_connector_state(state);
> +
> +	if (property == dev_priv->force_audio_property) {
> +		intel_conn_state->force_audio = val;
> +		return 0;
> +	}
> +
> +	if (property == dev_priv->broadcast_rgb_property) {
> +		intel_conn_state->broadcast_rgb = val;
> +		return 0;
> +	}
> +
> +	DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
> +	return -EINVAL;
> +}
> +
> +int intel_digital_connector_atomic_check(struct drm_connector *conn,
> +					 struct drm_connector_state *new_state)
> +{
> +	struct intel_digital_connector_state *new_conn_state =
> +		to_intel_digital_connector_state(new_state);
> +	struct drm_connector_state *old_state =
> +		drm_atomic_get_old_connector_state(new_state->state, conn);
> +	struct intel_digital_connector_state *old_conn_state =
> +		to_intel_digital_connector_state(old_state);
> +	struct intel_connector *intel_connector = to_intel_connector(conn);
> +	struct drm_crtc_state *crtc_state;
> +
> +	if (intel_connector->panel.allowed_fitting_mode_mask &&
> +	    (BIT(new_state->scaling_mode) & ~intel_connector->panel.allowed_fitting_mode_mask)) {
> +		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] Panel fitting mode %u not allowed.\n",
> +				 conn->base.id, conn->name, new_state->scaling_mode);
> +		return -EINVAL;
> +	}

Range-checking like this should be done by correctling registering the
proeprty ... Separate patch to fix that (if it's indeed broken)?

> +
> +	if (!new_state->crtc)
> +		return 0;
> +
> +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state, new_state->crtc);
> +
> +	/* Invisible to fastset since it's a pure connector property */
> +	if (new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
> +		crtc_state->connectors_changed = true;

Hm, I thought we put all the pfit register settings into the crtc state,
so scaling_mode changes should also be possible to handle by fastset?

Looks all reasonable otherwise.
-Daniel

> +
> +	/*
> +	 * These properties are handled by fastset, and might not end
> +	 * up in a modeset.
> +	 */
> +	if (new_conn_state->force_audio != old_conn_state->force_audio ||
> +	    new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
> +	    new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio)
> +		crtc_state->mode_changed = true;
> +
> +	return 0;
> +}
> +
> +/**
> + * intel_digital_connector_duplicate_state - duplicate connector state
> + * @connector: digital connector
> + *
> + * Allocates and returns a copy of the connector state (both common and
> + * digital connector specific) for the specified connector.
> + *
> + * Returns: The newly allocated connector state, or NULL on failure.
> + */
> +struct drm_connector_state *
> +intel_digital_connector_duplicate_state(struct drm_connector *connector)
> +{
> +	struct intel_digital_connector_state *state;
> +
> +	state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return NULL;
> +
> +	__drm_atomic_helper_connector_duplicate_state(connector, &state->base);
> +	return &state->base;
> +}
> +
> +/**
>   * intel_crtc_duplicate_state - duplicate crtc state
>   * @crtc: drm crtc
>   *
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 48a546210d8b..5eb4ddb04797 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5956,11 +5956,21 @@ static void intel_connector_verify_state(struct intel_connector *connector)
>  
>  int intel_connector_init(struct intel_connector *connector)
>  {
> -	drm_atomic_helper_connector_reset(&connector->base);
> +	struct intel_digital_connector_state *conn_state;
>  
> -	if (!connector->base.state)
> +	/*
> +	 * Allocate enough memory to hold intel_digital_connector_state,
> +	 * This might be a few bytes too many, but for connectors that don't
> +	 * need it we'll free the state and allocate a smaller one on the first
> +	 * succesful commit anyway.
> +	 */
> +	conn_state = kzalloc(sizeof(*conn_state), GFP_KERNEL);
> +	if (!conn_state)
>  		return -ENOMEM;
>  
> +	__drm_atomic_helper_connector_reset(&connector->base,
> +					    &conn_state->base);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 944b1449b7f9..0afd8f3037f0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5896,7 +5896,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  			      pipe_name(pipe));
>  	}
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>  	intel_panel_setup_backlight(connector, pipe);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f78d7c5f3805..4df9e4531edc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -262,6 +262,7 @@ struct intel_panel {
>  	struct drm_display_mode *fixed_mode;
>  	struct drm_display_mode *downclock_mode;
>  	int fitting_mode;
> +	unsigned allowed_fitting_mode_mask;
>  
>  	/* backlight */
>  	struct {
> @@ -323,6 +324,15 @@ struct intel_connector {
>  	struct intel_dp *mst_port;
>  };
>  
> +struct intel_digital_connector_state {
> +	struct drm_connector_state base;
> +
> +	enum hdmi_force_audio force_audio;
> +	int broadcast_rgb;
> +};
> +
> +#define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base)
> +
>  struct dpll {
>  	/* given values */
>  	int n;
> @@ -1668,7 +1678,8 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
>  /* intel_panel.c */
>  int intel_panel_init(struct intel_panel *panel,
>  		     struct drm_display_mode *fixed_mode,
> -		     struct drm_display_mode *downclock_mode);
> +		     struct drm_display_mode *downclock_mode,
> +		     unsigned allowed_fitting_mode_mask);
>  void intel_panel_fini(struct intel_panel *panel);
>  void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  			    struct drm_display_mode *adjusted_mode);
> @@ -1879,6 +1890,20 @@ int intel_connector_atomic_get_property(struct drm_connector *connector,
>  					const struct drm_connector_state *state,
>  					struct drm_property *property,
>  					uint64_t *val);
> +
> +int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
> +						const struct drm_connector_state *state,
> +						struct drm_property *property,
> +						uint64_t *val);
> +int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
> +						struct drm_connector_state *state,
> +						struct drm_property *property,
> +						uint64_t val);
> +int intel_digital_connector_atomic_check(struct drm_connector *conn,
> +					 struct drm_connector_state *new_state);
> +struct drm_connector_state *
> +intel_digital_connector_duplicate_state(struct drm_connector *connector);
> +
>  struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
>  void intel_crtc_destroy_state(struct drm_crtc *crtc,
>  			       struct drm_crtc_state *state);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3ffe8b1f1d48..e787142025ac 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1853,7 +1853,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
>  	connector->display_info.width_mm = fixed_mode->width_mm;
>  	connector->display_info.height_mm = fixed_mode->height_mm;
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> +	intel_panel_init(&intel_connector->panel, fixed_mode, NULL, 0);
>  	intel_panel_setup_backlight(connector, INVALID_PIPE);
>  
>  	intel_dsi_add_properties(intel_connector);
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index c1544a53095d..0b723157c38b 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -554,7 +554,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
>  			 */
>  			intel_panel_init(&intel_connector->panel,
>  					 intel_dvo_get_current_mode(connector),
> -					 NULL);
> +					 NULL, 0);
>  			intel_dvo->panel_wants_dither = true;
>  		}
>  
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 8b942ef2b3ec..919d84290774 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1181,7 +1181,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>  out:
>  	mutex_unlock(&dev->mode_config.mutex);
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
>  	intel_panel_setup_backlight(connector, INVALID_PIPE);
>  
>  	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index cb50c527401f..c6ce15550e5e 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1801,12 +1801,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>  
>  int intel_panel_init(struct intel_panel *panel,
>  		     struct drm_display_mode *fixed_mode,
> -		     struct drm_display_mode *downclock_mode)
> +		     struct drm_display_mode *downclock_mode,
> +		     unsigned allowed_fitting_mode_mask)
>  {
>  	intel_panel_init_backlight_funcs(panel);
>  
>  	panel->fixed_mode = fixed_mode;
>  	panel->downclock_mode = downclock_mode;
> +	panel->allowed_fitting_mode_mask = allowed_fitting_mode_mask;
>  
>  	return 0;
>  }
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 2/9] drm/i915: Add plumbing for digital connector state, v2.

[Maarten Lankhorst]

On 19-04-17 17:48, Daniel Vetter wrote:
> On Wed, Apr 12, 2017 at 12:50:00PM +0200, Maarten Lankhorst wrote:
>> Some atomic properties are common between the various kinds of
>> connectors, for example a lot of them use panel fitting mode.
>> It makes sense to put a lot of it in a common place, so each
>> connector can use it while they're being converted.
>>
>> Implement the properties required for the connectors:
>> - scaling mode property
>> - force audio property
>> - broadcast rgb
>> - aspect ratio
>>
>> While at it, make clear that intel_digital_connector_atomic_get_property
>> is a hack that has to be removed when all connector properties
>> are converted to atomic.
>>
>> Changes since v1:
>> - Scaling mode and aspect ratio are partly handled in core now.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_atomic.c  | 142 +++++++++++++++++++++++++++++++++--
>>  drivers/gpu/drm/i915/intel_display.c |  14 +++-
>>  drivers/gpu/drm/i915/intel_dp.c      |   2 +-
>>  drivers/gpu/drm/i915/intel_drv.h     |  27 ++++++-
>>  drivers/gpu/drm/i915/intel_dsi.c     |   2 +-
>>  drivers/gpu/drm/i915/intel_dvo.c     |   2 +-
>>  drivers/gpu/drm/i915/intel_lvds.c    |   2 +-
>>  drivers/gpu/drm/i915/intel_panel.c   |   4 +-
>>  8 files changed, 180 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index 50fb1f76cc5f..eb72166675f0 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> Refactoring idea for the future: intel_atomic.c makes 0 sense, just makes
> sure we have lots of functionality spread all over imo. I think following
> the split-up model I've done in the drm core would be a lot better, with
> intel_connector/encoder/crtc/plane.c for the shared helper functions. And
> then maybe separate files for the per-platform stuff, e.g. i9xx_crtc.c,
> pch_crtc.c and so on.
>
>> @@ -36,7 +36,7 @@
>>  #include "intel_drv.h"
>>  
>>  /**
>> - * intel_connector_atomic_get_property - fetch connector property value
>> + * intel_connector_atomic_get_property - fetch legacy connector property value
>>   * @connector: connector to fetch property for
>>   * @state: state containing the property value
>>   * @property: property to look up
>> @@ -45,12 +45,14 @@
>>   * The DRM core does not store shadow copies of properties for
>>   * atomic-capable drivers.  This entrypoint is used to fetch
>>   * the current value of a driver-specific connector property.
>> + *
>> + * This is a intermediary solution until all connectors are
>> + * converted to support full atomic properties.
>>   */
>> -int
>> -intel_connector_atomic_get_property(struct drm_connector *connector,
>> -				    const struct drm_connector_state *state,
>> -				    struct drm_property *property,
>> -				    uint64_t *val)
>> +int intel_connector_atomic_get_property(struct drm_connector *connector,
>> +					const struct drm_connector_state *state,
>> +					struct drm_property *property,
>> +					uint64_t *val)
>>  {
>>  	int i;
>>  
>> @@ -73,7 +75,133 @@ intel_connector_atomic_get_property(struct drm_connector *connector,
>>  	return -EINVAL;
>>  }
>>  
>> -/*
>> +/**
>> + * intel_digital_connector_atomic_get_property - hook for connector->atomic_get_property.
>> + * @connector: Connector to get the property for.
>> + * @state: Connector state to retrieve the property from.
>> + * @property: Property to retrieve.
>> + * @val: Return value for the property.
>> + *
>> + * Returns the atomic property value for a digital connector.
>> + */
>> +int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
>> +						const struct drm_connector_state *state,
>> +						struct drm_property *property,
>> +						uint64_t *val)
>> +{
>> +	struct drm_device *dev = connector->dev;
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	struct intel_digital_connector_state *intel_conn_state =
>> +		to_intel_digital_connector_state(state);
>> +
>> +	if (property == dev_priv->force_audio_property)
>> +		*val = intel_conn_state->force_audio;
>> +	else if (property == dev_priv->broadcast_rgb_property)
>> +		*val = intel_conn_state->broadcast_rgb;
>> +	else {
>> +		DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * intel_digital_connector_atomic_set_property - hook for connector->atomic_set_property.
>> + * @connector: Connector to set the property for.
>> + * @state: Connector state to set the property on.
>> + * @property: Property to set.
>> + * @val: New value for the property.
>> + *
>> + * Sets the atomic property value for a digital connector.
>> + */
>> +int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
>> +						struct drm_connector_state *state,
>> +						struct drm_property *property,
>> +						uint64_t val)
>> +{
>> +	struct drm_device *dev = connector->dev;
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	struct intel_digital_connector_state *intel_conn_state =
>> +		to_intel_digital_connector_state(state);
>> +
>> +	if (property == dev_priv->force_audio_property) {
>> +		intel_conn_state->force_audio = val;
>> +		return 0;
>> +	}
>> +
>> +	if (property == dev_priv->broadcast_rgb_property) {
>> +		intel_conn_state->broadcast_rgb = val;
>> +		return 0;
>> +	}
>> +
>> +	DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
>> +	return -EINVAL;
>> +}
>> +
>> +int intel_digital_connector_atomic_check(struct drm_connector *conn,
>> +					 struct drm_connector_state *new_state)
>> +{
>> +	struct intel_digital_connector_state *new_conn_state =
>> +		to_intel_digital_connector_state(new_state);
>> +	struct drm_connector_state *old_state =
>> +		drm_atomic_get_old_connector_state(new_state->state, conn);
>> +	struct intel_digital_connector_state *old_conn_state =
>> +		to_intel_digital_connector_state(old_state);
>> +	struct intel_connector *intel_connector = to_intel_connector(conn);
>> +	struct drm_crtc_state *crtc_state;
>> +
>> +	if (intel_connector->panel.allowed_fitting_mode_mask &&
>> +	    (BIT(new_state->scaling_mode) & ~intel_connector->panel.allowed_fitting_mode_mask)) {
>> +		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] Panel fitting mode %u not allowed.\n",
>> +				 conn->base.id, conn->name, new_state->scaling_mode);
>> +		return -EINVAL;
>> +	}
> Range-checking like this should be done by correctling registering the
> proeprty ... Separate patch to fix that (if it's indeed broken)?
Not if there's hw with a combination of eDP, DSI or LVDS. Even if it' s just prerelease testing hw.

This is matching the current behavior for setprop, so not much difference here. :)
>> +
>> +	if (!new_state->crtc)
>> +		return 0;
>> +
>> +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state, new_state->crtc);
>> +
>> +	/* Invisible to fastset since it's a pure connector property */
>> +	if (new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
>> +		crtc_state->connectors_changed = true;
> Hm, I thought we put all the pfit register settings into the crtc state,
> so scaling_mode changes should also be possible to handle by fastset?
Quite possible, I didn't check if it was the case though, so I erred on the side of caution.
> Looks all reasonable otherwise.
> -Daniel
>
>> +
>> +	/*
>> +	 * These properties are handled by fastset, and might not end
>> +	 * up in a modeset.
>> +	 */
>> +	if (new_conn_state->force_audio != old_conn_state->force_audio ||
>> +	    new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
>> +	    new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio)
>> +		crtc_state->mode_changed = true;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * intel_digital_connector_duplicate_state - duplicate connector state
>> + * @connector: digital connector
>> + *
>> + * Allocates and returns a copy of the connector state (both common and
>> + * digital connector specific) for the specified connector.
>> + *
>> + * Returns: The newly allocated connector state, or NULL on failure.
>> + */
>> +struct drm_connector_state *
>> +intel_digital_connector_duplicate_state(struct drm_connector *connector)
>> +{
>> +	struct intel_digital_connector_state *state;
>> +
>> +	state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
>> +	if (!state)
>> +		return NULL;
>> +
>> +	__drm_atomic_helper_connector_duplicate_state(connector, &state->base);
>> +	return &state->base;
>> +}
>> +
>> +/**
>>   * intel_crtc_duplicate_state - duplicate crtc state
>>   * @crtc: drm crtc
>>   *
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 48a546210d8b..5eb4ddb04797 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5956,11 +5956,21 @@ static void intel_connector_verify_state(struct intel_connector *connector)
>>  
>>  int intel_connector_init(struct intel_connector *connector)
>>  {
>> -	drm_atomic_helper_connector_reset(&connector->base);
>> +	struct intel_digital_connector_state *conn_state;
>>  
>> -	if (!connector->base.state)
>> +	/*
>> +	 * Allocate enough memory to hold intel_digital_connector_state,
>> +	 * This might be a few bytes too many, but for connectors that don't
>> +	 * need it we'll free the state and allocate a smaller one on the first
>> +	 * succesful commit anyway.
>> +	 */
>> +	conn_state = kzalloc(sizeof(*conn_state), GFP_KERNEL);
>> +	if (!conn_state)
>>  		return -ENOMEM;
>>  
>> +	__drm_atomic_helper_connector_reset(&connector->base,
>> +					    &conn_state->base);
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 944b1449b7f9..0afd8f3037f0 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5896,7 +5896,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  			      pipe_name(pipe));
>>  	}
>>  
>> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
>>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>>  	intel_panel_setup_backlight(connector, pipe);
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index f78d7c5f3805..4df9e4531edc 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -262,6 +262,7 @@ struct intel_panel {
>>  	struct drm_display_mode *fixed_mode;
>>  	struct drm_display_mode *downclock_mode;
>>  	int fitting_mode;
>> +	unsigned allowed_fitting_mode_mask;
>>  
>>  	/* backlight */
>>  	struct {
>> @@ -323,6 +324,15 @@ struct intel_connector {
>>  	struct intel_dp *mst_port;
>>  };
>>  
>> +struct intel_digital_connector_state {
>> +	struct drm_connector_state base;
>> +
>> +	enum hdmi_force_audio force_audio;
>> +	int broadcast_rgb;
>> +};
>> +
>> +#define to_intel_digital_connector_state(x) container_of(x, struct intel_digital_connector_state, base)
>> +
>>  struct dpll {
>>  	/* given values */
>>  	int n;
>> @@ -1668,7 +1678,8 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
>>  /* intel_panel.c */
>>  int intel_panel_init(struct intel_panel *panel,
>>  		     struct drm_display_mode *fixed_mode,
>> -		     struct drm_display_mode *downclock_mode);
>> +		     struct drm_display_mode *downclock_mode,
>> +		     unsigned allowed_fitting_mode_mask);
>>  void intel_panel_fini(struct intel_panel *panel);
>>  void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>>  			    struct drm_display_mode *adjusted_mode);
>> @@ -1879,6 +1890,20 @@ int intel_connector_atomic_get_property(struct drm_connector *connector,
>>  					const struct drm_connector_state *state,
>>  					struct drm_property *property,
>>  					uint64_t *val);
>> +
>> +int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
>> +						const struct drm_connector_state *state,
>> +						struct drm_property *property,
>> +						uint64_t *val);
>> +int intel_digital_connector_atomic_set_property(struct drm_connector *connector,
>> +						struct drm_connector_state *state,
>> +						struct drm_property *property,
>> +						uint64_t val);
>> +int intel_digital_connector_atomic_check(struct drm_connector *conn,
>> +					 struct drm_connector_state *new_state);
>> +struct drm_connector_state *
>> +intel_digital_connector_duplicate_state(struct drm_connector *connector);
>> +
>>  struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
>>  void intel_crtc_destroy_state(struct drm_crtc *crtc,
>>  			       struct drm_crtc_state *state);
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 3ffe8b1f1d48..e787142025ac 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -1853,7 +1853,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
>>  	connector->display_info.width_mm = fixed_mode->width_mm;
>>  	connector->display_info.height_mm = fixed_mode->height_mm;
>>  
>> -	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>> +	intel_panel_init(&intel_connector->panel, fixed_mode, NULL, 0);
>>  	intel_panel_setup_backlight(connector, INVALID_PIPE);
>>  
>>  	intel_dsi_add_properties(intel_connector);
>> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
>> index c1544a53095d..0b723157c38b 100644
>> --- a/drivers/gpu/drm/i915/intel_dvo.c
>> +++ b/drivers/gpu/drm/i915/intel_dvo.c
>> @@ -554,7 +554,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
>>  			 */
>>  			intel_panel_init(&intel_connector->panel,
>>  					 intel_dvo_get_current_mode(connector),
>> -					 NULL);
>> +					 NULL, 0);
>>  			intel_dvo->panel_wants_dither = true;
>>  		}
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>> index 8b942ef2b3ec..919d84290774 100644
>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> @@ -1181,7 +1181,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>>  out:
>>  	mutex_unlock(&dev->mode_config.mutex);
>>  
>> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
>>  	intel_panel_setup_backlight(connector, INVALID_PIPE);
>>  
>>  	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index cb50c527401f..c6ce15550e5e 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -1801,12 +1801,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>>  
>>  int intel_panel_init(struct intel_panel *panel,
>>  		     struct drm_display_mode *fixed_mode,
>> -		     struct drm_display_mode *downclock_mode)
>> +		     struct drm_display_mode *downclock_mode,
>> +		     unsigned allowed_fitting_mode_mask)
>>  {
>>  	intel_panel_init_backlight_funcs(panel);
>>  
>>  	panel->fixed_mode = fixed_mode;
>>  	panel->downclock_mode = downclock_mode;
>> +	panel->allowed_fitting_mode_mask = allowed_fitting_mode_mask;
>>  
>>  	return 0;
>>  }
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

[PATCH v4 3/9] drm/i915: Convert DSI connector properties to atomic.

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 57 +++++++---------------------------------
 1 file changed, 9 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index e787142025ac..1d8621538563 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -320,10 +320,10 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 
 		if (HAS_GMCH_DISPLAY(dev_priv))
 			intel_gmch_panel_fitting(crtc, pipe_config,
-						 intel_connector->panel.fitting_mode);
+						 conn_state->scaling_mode);
 		else
 			intel_pch_panel_fitting(crtc, pipe_config,
-						intel_connector->panel.fitting_mode);
+						conn_state->scaling_mode);
 	}
 
 	/* DSI uses short packets for sync events, so clear mode flags for DSI */
@@ -1588,48 +1588,6 @@ static int intel_dsi_get_modes(struct drm_connector *connector)
 	return 1;
 }
 
-static int intel_dsi_set_property(struct drm_connector *connector,
-				  struct drm_property *property,
-				  uint64_t val)
-{
-	struct drm_device *dev = connector->dev;
-	struct intel_connector *intel_connector = to_intel_connector(connector);
-	struct drm_crtc *crtc;
-	int ret;
-
-	ret = drm_object_property_set_value(&connector->base, property, val);
-	if (ret)
-		return ret;
-
-	if (property == dev->mode_config.scaling_mode_property) {
-		if (val == DRM_MODE_SCALE_NONE) {
-			DRM_DEBUG_KMS("no scaling not supported\n");
-			return -EINVAL;
-		}
-		if (HAS_GMCH_DISPLAY(to_i915(dev)) &&
-		    val == DRM_MODE_SCALE_CENTER) {
-			DRM_DEBUG_KMS("centering not supported\n");
-			return -EINVAL;
-		}
-
-		if (intel_connector->panel.fitting_mode == val)
-			return 0;
-
-		intel_connector->panel.fitting_mode = val;
-	}
-
-	crtc = connector->state->crtc;
-	if (crtc && crtc->state->enable) {
-		/*
-		 * If the CRTC is enabled, the display will be changed
-		 * according to the new panel fitting mode.
-		 */
-		intel_crtc_restore_mode(crtc);
-	}
-
-	return 0;
-}
-
 static void intel_dsi_connector_destroy(struct drm_connector *connector)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
@@ -1658,6 +1616,7 @@ static const struct drm_encoder_funcs intel_dsi_funcs = {
 static const struct drm_connector_helper_funcs intel_dsi_connector_helper_funcs = {
 	.get_modes = intel_dsi_get_modes,
 	.mode_valid = intel_dsi_mode_valid,
+	.atomic_check = intel_digital_connector_atomic_check,
 };
 
 static const struct drm_connector_funcs intel_dsi_connector_funcs = {
@@ -1666,10 +1625,11 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_dsi_connector_destroy,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = intel_dsi_set_property,
-	.atomic_get_property = intel_connector_atomic_get_property,
+	.set_property = drm_atomic_helper_connector_set_property,
+	.atomic_get_property = intel_digital_connector_atomic_get_property,
+	.atomic_set_property = intel_digital_connector_atomic_set_property,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
 };
 
 static void intel_dsi_add_properties(struct intel_connector *connector)
@@ -1681,7 +1641,8 @@ static void intel_dsi_add_properties(struct intel_connector *connector)
 		drm_object_attach_property(&connector->base.base,
 					   dev->mode_config.scaling_mode_property,
 					   DRM_MODE_SCALE_ASPECT);
-		connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
+
+		connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
 	}
 }
 
-- 
2.7.4

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

[PATCH v4 4/9] drm/i915: Convert LVDS connector properties to atomic.

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lvds.c | 56 +++++++++++----------------------------
 1 file changed, 15 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 919d84290774..39d983d9b702 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -401,6 +401,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 		&lvds_encoder->attached_connector->base;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
+
 	unsigned int lvds_bpp;
 
 	/* Should never happen!! */
@@ -433,10 +434,10 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 		pipe_config->has_pch_encoder = true;
 
 		intel_pch_panel_fitting(intel_crtc, pipe_config,
-					intel_connector->panel.fitting_mode);
+					conn_state->scaling_mode);
 	} else {
 		intel_gmch_panel_fitting(intel_crtc, pipe_config,
-					 intel_connector->panel.fitting_mode);
+					 conn_state->scaling_mode);
 
 	}
 
@@ -598,56 +599,24 @@ static void intel_lvds_destroy(struct drm_connector *connector)
 	kfree(connector);
 }
 
-static int intel_lvds_set_property(struct drm_connector *connector,
-				   struct drm_property *property,
-				   uint64_t value)
-{
-	struct intel_connector *intel_connector = to_intel_connector(connector);
-	struct drm_device *dev = connector->dev;
-
-	if (property == dev->mode_config.scaling_mode_property) {
-		struct drm_crtc *crtc;
-
-		if (value == DRM_MODE_SCALE_NONE) {
-			DRM_DEBUG_KMS("no scaling not supported\n");
-			return -EINVAL;
-		}
-
-		if (intel_connector->panel.fitting_mode == value) {
-			/* the LVDS scaling property is not changed */
-			return 0;
-		}
-		intel_connector->panel.fitting_mode = value;
-
-		crtc = intel_attached_encoder(connector)->base.crtc;
-		if (crtc && crtc->state->enable) {
-			/*
-			 * If the CRTC is enabled, the display will be changed
-			 * according to the new panel fitting mode.
-			 */
-			intel_crtc_restore_mode(crtc);
-		}
-	}
-
-	return 0;
-}
-
 static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
 	.get_modes = intel_lvds_get_modes,
 	.mode_valid = intel_lvds_mode_valid,
+	.atomic_check = intel_digital_connector_atomic_check,
 };
 
 static const struct drm_connector_funcs intel_lvds_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_lvds_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = intel_lvds_set_property,
-	.atomic_get_property = intel_connector_atomic_get_property,
+	.set_property = drm_atomic_helper_connector_set_property,
+	.atomic_get_property = intel_digital_connector_atomic_get_property,
+	.atomic_set_property = intel_digital_connector_atomic_set_property,
 	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_lvds_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
 };
 
 static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
@@ -988,6 +957,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 	u32 lvds;
 	int pipe;
 	u8 pin;
+	unsigned allowed_fittings;
 
 	if (!intel_lvds_supported(dev_priv))
 		return;
@@ -1087,7 +1057,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 	drm_object_attach_property(&connector->base,
 				      dev->mode_config.scaling_mode_property,
 				      DRM_MODE_SCALE_ASPECT);
-	intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
+	connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
 
 	intel_lvds_pps_get_hw_state(dev_priv, &lvds_encoder->init_pps);
 	lvds_encoder->init_lvds_val = lvds;
@@ -1181,7 +1151,11 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 out:
 	mutex_unlock(&dev->mode_config.mutex);
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
+	allowed_fittings = BIT(DRM_MODE_SCALE_CENTER);
+	allowed_fittings |= BIT(DRM_MODE_SCALE_ASPECT);
+	allowed_fittings |= BIT(DRM_MODE_SCALE_FULLSCREEN);
+
+	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, allowed_fittings);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
-- 
2.7.4

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

[PATCH v4 5/9] drm/i915: Make intel_dp->has_audio reflect hw state only

[Maarten Lankhorst]

Always detect if audio is available during edid detection.
With less magic switching it's easier to convert the dp connector
properties to atomic.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0afd8f3037f0..b1a0cb3c79d4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1653,7 +1653,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		pipe_config->has_pch_encoder = true;
 
 	pipe_config->has_drrs = false;
-	pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
+	if (port == PORT_A)
+		pipe_config->has_audio = false;
+	else if (intel_dp->force_audio == HDMI_AUDIO_AUTO)
+		pipe_config->has_audio = intel_dp->has_audio;
+	else
+		pipe_config->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
 
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
@@ -4578,10 +4583,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	edid = intel_dp_get_edid(intel_dp);
 	intel_connector->detect_edid = edid;
 
-	if (intel_dp->force_audio != HDMI_AUDIO_AUTO)
-		intel_dp->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
-	else
-		intel_dp->has_audio = drm_detect_monitor_audio(edid);
+	intel_dp->has_audio = drm_detect_monitor_audio(edid);
 }
 
 static void
@@ -4778,19 +4780,6 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 	return 0;
 }
 
-static bool
-intel_dp_detect_audio(struct drm_connector *connector)
-{
-	bool has_audio = false;
-	struct edid *edid;
-
-	edid = to_intel_connector(connector)->detect_edid;
-	if (edid)
-		has_audio = drm_detect_monitor_audio(edid);
-
-	return has_audio;
-}
-
 static int
 intel_dp_set_property(struct drm_connector *connector,
 		      struct drm_property *property,
@@ -4808,22 +4797,27 @@ intel_dp_set_property(struct drm_connector *connector,
 
 	if (property == dev_priv->force_audio_property) {
 		int i = val;
-		bool has_audio;
+		bool has_audio, old_has_audio;
+		int old_force_audio = intel_dp->force_audio;
 
 		if (i == intel_dp->force_audio)
 			return 0;
 
+		if (old_force_audio == HDMI_AUDIO_AUTO)
+			old_has_audio = intel_dp->has_audio;
+		else
+			old_has_audio = old_force_audio;
+
 		intel_dp->force_audio = i;
 
 		if (i == HDMI_AUDIO_AUTO)
-			has_audio = intel_dp_detect_audio(connector);
+			has_audio = intel_dp->has_audio;
 		else
 			has_audio = (i == HDMI_AUDIO_ON);
 
-		if (has_audio == intel_dp->has_audio)
+		if (has_audio == old_has_audio)
 			return 0;
 
-		intel_dp->has_audio = has_audio;
 		goto done;
 	}
 
-- 
2.7.4

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

[PATCH v4 6/9] drm/i915: Convert intel_dp properties to atomic.

[Maarten Lankhorst]

intel_dp supports 3 properties, scaling mode, broadcast rgb and
force_audio. intel_digital_connector handles the plumbing,
so we only have to hook this up in compute_config and init.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 136 +++++++--------------------------------
 drivers/gpu/drm/i915/intel_drv.h |   3 -
 2 files changed, 24 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b1a0cb3c79d4..f976d10b4f0a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1630,6 +1630,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct intel_digital_connector_state *intel_conn_state =
+		to_intel_digital_connector_state(conn_state);
 	int lane_count, clock;
 	int min_lane_count = 1;
 	int max_lane_count = intel_dp_max_lane_count(intel_dp);
@@ -1655,10 +1657,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	pipe_config->has_drrs = false;
 	if (port == PORT_A)
 		pipe_config->has_audio = false;
-	else if (intel_dp->force_audio == HDMI_AUDIO_AUTO)
+	else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
 		pipe_config->has_audio = intel_dp->has_audio;
 	else
-		pipe_config->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
+		pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
 
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
@@ -1673,10 +1675,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 		if (HAS_GMCH_DISPLAY(dev_priv))
 			intel_gmch_panel_fitting(intel_crtc, pipe_config,
-						 intel_connector->panel.fitting_mode);
+						 conn_state->scaling_mode);
 		else
 			intel_pch_panel_fitting(intel_crtc, pipe_config,
-						intel_connector->panel.fitting_mode);
+						conn_state->scaling_mode);
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
@@ -1745,7 +1747,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	return false;
 
 found:
-	if (intel_dp->color_range_auto) {
+	if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
 		/*
 		 * See:
 		 * CEA-861-E - 5.1 Default Encoding Parameters
@@ -1757,7 +1759,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			HDMI_QUANTIZATION_RANGE_LIMITED;
 	} else {
 		pipe_config->limited_color_range =
-			intel_dp->limited_color_range;
+			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
 	}
 
 	pipe_config->lane_count = lane_count;
@@ -4781,104 +4783,6 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 }
 
 static int
-intel_dp_set_property(struct drm_connector *connector,
-		      struct drm_property *property,
-		      uint64_t val)
-{
-	struct drm_i915_private *dev_priv = to_i915(connector->dev);
-	struct intel_connector *intel_connector = to_intel_connector(connector);
-	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
-	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
-	int ret;
-
-	ret = drm_object_property_set_value(&connector->base, property, val);
-	if (ret)
-		return ret;
-
-	if (property == dev_priv->force_audio_property) {
-		int i = val;
-		bool has_audio, old_has_audio;
-		int old_force_audio = intel_dp->force_audio;
-
-		if (i == intel_dp->force_audio)
-			return 0;
-
-		if (old_force_audio == HDMI_AUDIO_AUTO)
-			old_has_audio = intel_dp->has_audio;
-		else
-			old_has_audio = old_force_audio;
-
-		intel_dp->force_audio = i;
-
-		if (i == HDMI_AUDIO_AUTO)
-			has_audio = intel_dp->has_audio;
-		else
-			has_audio = (i == HDMI_AUDIO_ON);
-
-		if (has_audio == old_has_audio)
-			return 0;
-
-		goto done;
-	}
-
-	if (property == dev_priv->broadcast_rgb_property) {
-		bool old_auto = intel_dp->color_range_auto;
-		bool old_range = intel_dp->limited_color_range;
-
-		switch (val) {
-		case INTEL_BROADCAST_RGB_AUTO:
-			intel_dp->color_range_auto = true;
-			break;
-		case INTEL_BROADCAST_RGB_FULL:
-			intel_dp->color_range_auto = false;
-			intel_dp->limited_color_range = false;
-			break;
-		case INTEL_BROADCAST_RGB_LIMITED:
-			intel_dp->color_range_auto = false;
-			intel_dp->limited_color_range = true;
-			break;
-		default:
-			return -EINVAL;
-		}
-
-		if (old_auto == intel_dp->color_range_auto &&
-		    old_range == intel_dp->limited_color_range)
-			return 0;
-
-		goto done;
-	}
-
-	if (is_edp(intel_dp) &&
-	    property == connector->dev->mode_config.scaling_mode_property) {
-		if (val == DRM_MODE_SCALE_NONE) {
-			DRM_DEBUG_KMS("no scaling not supported\n");
-			return -EINVAL;
-		}
-		if (HAS_GMCH_DISPLAY(dev_priv) &&
-		    val == DRM_MODE_SCALE_CENTER) {
-			DRM_DEBUG_KMS("centering not supported\n");
-			return -EINVAL;
-		}
-
-		if (intel_connector->panel.fitting_mode == val) {
-			/* the eDP scaling property is not changed */
-			return 0;
-		}
-		intel_connector->panel.fitting_mode = val;
-
-		goto done;
-	}
-
-	return -EINVAL;
-
-done:
-	if (intel_encoder->base.crtc)
-		intel_crtc_restore_mode(intel_encoder->base.crtc);
-
-	return 0;
-}
-
-static int
 intel_dp_connector_register(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -5036,19 +4940,21 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.force = intel_dp_force,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = intel_dp_set_property,
-	.atomic_get_property = intel_connector_atomic_get_property,
+	.set_property = drm_atomic_helper_connector_set_property,
+	.atomic_get_property = intel_digital_connector_atomic_get_property,
+	.atomic_set_property = intel_digital_connector_atomic_set_property,
 	.late_register = intel_dp_connector_register,
 	.early_unregister = intel_dp_connector_unregister,
 	.destroy = intel_dp_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
 };
 
 static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
 	.detect_ctx = intel_dp_detect,
 	.get_modes = intel_dp_get_modes,
 	.mode_valid = intel_dp_mode_valid,
+	.atomic_check = intel_digital_connector_atomic_check,
 };
 
 static const struct drm_encoder_funcs intel_dp_enc_funcs = {
@@ -5142,11 +5048,8 @@ bool intel_dp_is_edp(struct drm_i915_private *dev_priv, enum port port)
 static void
 intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector)
 {
-	struct intel_connector *intel_connector = to_intel_connector(connector);
-
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
-	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
 		drm_mode_create_scaling_mode_property(connector->dev);
@@ -5154,7 +5057,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 			&connector->base,
 			connector->dev->mode_config.scaling_mode_property,
 			DRM_MODE_SCALE_ASPECT);
-		intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
+
+		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
 	}
 }
 
@@ -5797,6 +5701,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	struct drm_display_mode *scan;
 	struct edid *edid;
 	enum pipe pipe = INVALID_PIPE;
+	unsigned allowed_fittings;
 
 	if (!is_edp(intel_dp))
 		return true;
@@ -5890,7 +5795,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 			      pipe_name(pipe));
 	}
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
+	allowed_fittings = 0;
+	if (!HAS_GMCH_DISPLAY(dev_priv))
+		allowed_fittings |= BIT(DRM_MODE_SCALE_CENTER);
+	allowed_fittings |= BIT(DRM_MODE_SCALE_ASPECT);
+	allowed_fittings |= BIT(DRM_MODE_SCALE_FULLSCREEN);
+
+	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode,
+			 allowed_fittings);
 	intel_connector->panel.backlight.power = intel_edp_backlight_power;
 	intel_panel_setup_backlight(connector, pipe);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4df9e4531edc..9f0f80203472 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -261,7 +261,6 @@ struct intel_encoder {
 struct intel_panel {
 	struct drm_display_mode *fixed_mode;
 	struct drm_display_mode *downclock_mode;
-	int fitting_mode;
 	unsigned allowed_fitting_mode_mask;
 
 	/* backlight */
@@ -952,9 +951,7 @@ struct intel_dp {
 	bool detect_done;
 	bool channel_eq_status;
 	bool reset_link_params;
-	enum hdmi_force_audio force_audio;
 	bool limited_color_range;
-	bool color_range_auto;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
 	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
-- 
2.7.4

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

Re: [PATCH v4 6/9] drm/i915: Convert intel_dp properties to atomic.

[Daniel Vetter]

On Wed, Apr 12, 2017 at 12:50:04PM +0200, Maarten Lankhorst wrote:
> intel_dp supports 3 properties, scaling mode, broadcast rgb and
> force_audio. intel_digital_connector handles the plumbing,
> so we only have to hook this up in compute_config and init.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 136 +++++++--------------------------------
>  drivers/gpu/drm/i915/intel_drv.h |   3 -
>  2 files changed, 24 insertions(+), 115 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b1a0cb3c79d4..f976d10b4f0a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1630,6 +1630,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct intel_digital_connector_state *intel_conn_state =
> +		to_intel_digital_connector_state(conn_state);
>  	int lane_count, clock;
>  	int min_lane_count = 1;
>  	int max_lane_count = intel_dp_max_lane_count(intel_dp);
> @@ -1655,10 +1657,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	pipe_config->has_drrs = false;
>  	if (port == PORT_A)
>  		pipe_config->has_audio = false;
> -	else if (intel_dp->force_audio == HDMI_AUDIO_AUTO)
> +	else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
>  		pipe_config->has_audio = intel_dp->has_audio;
>  	else
> -		pipe_config->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
> +		pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
>  
>  	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> @@ -1673,10 +1675,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  
>  		if (HAS_GMCH_DISPLAY(dev_priv))
>  			intel_gmch_panel_fitting(intel_crtc, pipe_config,
> -						 intel_connector->panel.fitting_mode);
> +						 conn_state->scaling_mode);
>  		else
>  			intel_pch_panel_fitting(intel_crtc, pipe_config,
> -						intel_connector->panel.fitting_mode);
> +						conn_state->scaling_mode);
>  	}
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> @@ -1745,7 +1747,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	return false;
>  
>  found:
> -	if (intel_dp->color_range_auto) {
> +	if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
>  		/*
>  		 * See:
>  		 * CEA-861-E - 5.1 Default Encoding Parameters
> @@ -1757,7 +1759,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			HDMI_QUANTIZATION_RANGE_LIMITED;
>  	} else {
>  		pipe_config->limited_color_range =
> -			intel_dp->limited_color_range;
> +			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
>  	}
>  
>  	pipe_config->lane_count = lane_count;
> @@ -4781,104 +4783,6 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  }
>  
>  static int
> -intel_dp_set_property(struct drm_connector *connector,
> -		      struct drm_property *property,
> -		      uint64_t val)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> -	struct intel_connector *intel_connector = to_intel_connector(connector);
> -	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
> -	int ret;
> -
> -	ret = drm_object_property_set_value(&connector->base, property, val);
> -	if (ret)
> -		return ret;
> -
> -	if (property == dev_priv->force_audio_property) {
> -		int i = val;
> -		bool has_audio, old_has_audio;
> -		int old_force_audio = intel_dp->force_audio;
> -
> -		if (i == intel_dp->force_audio)
> -			return 0;
> -
> -		if (old_force_audio == HDMI_AUDIO_AUTO)
> -			old_has_audio = intel_dp->has_audio;
> -		else
> -			old_has_audio = old_force_audio;
> -
> -		intel_dp->force_audio = i;
> -
> -		if (i == HDMI_AUDIO_AUTO)
> -			has_audio = intel_dp->has_audio;
> -		else
> -			has_audio = (i == HDMI_AUDIO_ON);
> -
> -		if (has_audio == old_has_audio)
> -			return 0;
> -
> -		goto done;
> -	}
> -
> -	if (property == dev_priv->broadcast_rgb_property) {
> -		bool old_auto = intel_dp->color_range_auto;
> -		bool old_range = intel_dp->limited_color_range;
> -
> -		switch (val) {
> -		case INTEL_BROADCAST_RGB_AUTO:
> -			intel_dp->color_range_auto = true;
> -			break;
> -		case INTEL_BROADCAST_RGB_FULL:
> -			intel_dp->color_range_auto = false;
> -			intel_dp->limited_color_range = false;
> -			break;
> -		case INTEL_BROADCAST_RGB_LIMITED:
> -			intel_dp->color_range_auto = false;
> -			intel_dp->limited_color_range = true;
> -			break;
> -		default:
> -			return -EINVAL;
> -		}
> -
> -		if (old_auto == intel_dp->color_range_auto &&
> -		    old_range == intel_dp->limited_color_range)
> -			return 0;
> -
> -		goto done;
> -	}
> -
> -	if (is_edp(intel_dp) &&
> -	    property == connector->dev->mode_config.scaling_mode_property) {
> -		if (val == DRM_MODE_SCALE_NONE) {
> -			DRM_DEBUG_KMS("no scaling not supported\n");
> -			return -EINVAL;
> -		}
> -		if (HAS_GMCH_DISPLAY(dev_priv) &&
> -		    val == DRM_MODE_SCALE_CENTER) {
> -			DRM_DEBUG_KMS("centering not supported\n");
> -			return -EINVAL;
> -		}
> -
> -		if (intel_connector->panel.fitting_mode == val) {
> -			/* the eDP scaling property is not changed */
> -			return 0;
> -		}
> -		intel_connector->panel.fitting_mode = val;
> -
> -		goto done;
> -	}
> -
> -	return -EINVAL;
> -
> -done:
> -	if (intel_encoder->base.crtc)
> -		intel_crtc_restore_mode(intel_encoder->base.crtc);
> -
> -	return 0;
> -}
> -
> -static int
>  intel_dp_connector_register(struct drm_connector *connector)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> @@ -5036,19 +4940,21 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
>  	.dpms = drm_atomic_helper_connector_dpms,
>  	.force = intel_dp_force,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.set_property = intel_dp_set_property,
> -	.atomic_get_property = intel_connector_atomic_get_property,
> +	.set_property = drm_atomic_helper_connector_set_property,
> +	.atomic_get_property = intel_digital_connector_atomic_get_property,
> +	.atomic_set_property = intel_digital_connector_atomic_set_property,
>  	.late_register = intel_dp_connector_register,
>  	.early_unregister = intel_dp_connector_unregister,
>  	.destroy = intel_dp_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>  };
>  
>  static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
>  	.detect_ctx = intel_dp_detect,
>  	.get_modes = intel_dp_get_modes,
>  	.mode_valid = intel_dp_mode_valid,
> +	.atomic_check = intel_digital_connector_atomic_check,
>  };
>  
>  static const struct drm_encoder_funcs intel_dp_enc_funcs = {
> @@ -5142,11 +5048,8 @@ bool intel_dp_is_edp(struct drm_i915_private *dev_priv, enum port port)
>  static void
>  intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector)
>  {
> -	struct intel_connector *intel_connector = to_intel_connector(connector);
> -
>  	intel_attach_force_audio_property(connector);
>  	intel_attach_broadcast_rgb_property(connector);
> -	intel_dp->color_range_auto = true;
>  
>  	if (is_edp(intel_dp)) {
>  		drm_mode_create_scaling_mode_property(connector->dev);
> @@ -5154,7 +5057,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  			&connector->base,
>  			connector->dev->mode_config.scaling_mode_property,
>  			DRM_MODE_SCALE_ASPECT);
> -		intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
> +
> +		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  	}
>  }
>  
> @@ -5797,6 +5701,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	struct drm_display_mode *scan;
>  	struct edid *edid;
>  	enum pipe pipe = INVALID_PIPE;
> +	unsigned allowed_fittings;
>  
>  	if (!is_edp(intel_dp))
>  		return true;
> @@ -5890,7 +5795,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  			      pipe_name(pipe));
>  	}
>  
> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
> +	allowed_fittings = 0;
> +	if (!HAS_GMCH_DISPLAY(dev_priv))
> +		allowed_fittings |= BIT(DRM_MODE_SCALE_CENTER);
> +	allowed_fittings |= BIT(DRM_MODE_SCALE_ASPECT);
> +	allowed_fittings |= BIT(DRM_MODE_SCALE_FULLSCREEN);
> +
> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode,
> +			 allowed_fittings);
>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>  	intel_panel_setup_backlight(connector, pipe);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4df9e4531edc..9f0f80203472 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -261,7 +261,6 @@ struct intel_encoder {
>  struct intel_panel {
>  	struct drm_display_mode *fixed_mode;
>  	struct drm_display_mode *downclock_mode;
> -	int fitting_mode;
>  	unsigned allowed_fitting_mode_mask;
>  
>  	/* backlight */
> @@ -952,9 +951,7 @@ struct intel_dp {
>  	bool detect_done;
>  	bool channel_eq_status;
>  	bool reset_link_params;
> -	enum hdmi_force_audio force_audio;
>  	bool limited_color_range;

Can't we remove limited_color_range now too? Otherwise lgtm.
-Daniel


> -	bool color_range_auto;
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
>  	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 6/9] drm/i915: Convert intel_dp properties to atomic.

[Maarten Lankhorst]

On 19-04-17 17:53, Daniel Vetter wrote:
> On Wed, Apr 12, 2017 at 12:50:04PM +0200, Maarten Lankhorst wrote:
>> intel_dp supports 3 properties, scaling mode, broadcast rgb and
>> force_audio. intel_digital_connector handles the plumbing,
>> so we only have to hook this up in compute_config and init.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c  | 136 +++++++--------------------------------
>>  drivers/gpu/drm/i915/intel_drv.h |   3 -
>>  2 files changed, 24 insertions(+), 115 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index b1a0cb3c79d4..f976d10b4f0a 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1630,6 +1630,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  	enum port port = dp_to_dig_port(intel_dp)->port;
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
>> +	struct intel_digital_connector_state *intel_conn_state =
>> +		to_intel_digital_connector_state(conn_state);
>>  	int lane_count, clock;
>>  	int min_lane_count = 1;
>>  	int max_lane_count = intel_dp_max_lane_count(intel_dp);
>> @@ -1655,10 +1657,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  	pipe_config->has_drrs = false;
>>  	if (port == PORT_A)
>>  		pipe_config->has_audio = false;
>> -	else if (intel_dp->force_audio == HDMI_AUDIO_AUTO)
>> +	else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
>>  		pipe_config->has_audio = intel_dp->has_audio;
>>  	else
>> -		pipe_config->has_audio = intel_dp->force_audio == HDMI_AUDIO_ON;
>> +		pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
>>  
>>  	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>>  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>> @@ -1673,10 +1675,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  
>>  		if (HAS_GMCH_DISPLAY(dev_priv))
>>  			intel_gmch_panel_fitting(intel_crtc, pipe_config,
>> -						 intel_connector->panel.fitting_mode);
>> +						 conn_state->scaling_mode);
>>  		else
>>  			intel_pch_panel_fitting(intel_crtc, pipe_config,
>> -						intel_connector->panel.fitting_mode);
>> +						conn_state->scaling_mode);
>>  	}
>>  
>>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>> @@ -1745,7 +1747,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  	return false;
>>  
>>  found:
>> -	if (intel_dp->color_range_auto) {
>> +	if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
>>  		/*
>>  		 * See:
>>  		 * CEA-861-E - 5.1 Default Encoding Parameters
>> @@ -1757,7 +1759,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>  			HDMI_QUANTIZATION_RANGE_LIMITED;
>>  	} else {
>>  		pipe_config->limited_color_range =
>> -			intel_dp->limited_color_range;
>> +			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
>>  	}
>>  
>>  	pipe_config->lane_count = lane_count;
>> @@ -4781,104 +4783,6 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>>  }
>>  
>>  static int
>> -intel_dp_set_property(struct drm_connector *connector,
>> -		      struct drm_property *property,
>> -		      uint64_t val)
>> -{
>> -	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> -	struct intel_connector *intel_connector = to_intel_connector(connector);
>> -	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
>> -	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
>> -	int ret;
>> -
>> -	ret = drm_object_property_set_value(&connector->base, property, val);
>> -	if (ret)
>> -		return ret;
>> -
>> -	if (property == dev_priv->force_audio_property) {
>> -		int i = val;
>> -		bool has_audio, old_has_audio;
>> -		int old_force_audio = intel_dp->force_audio;
>> -
>> -		if (i == intel_dp->force_audio)
>> -			return 0;
>> -
>> -		if (old_force_audio == HDMI_AUDIO_AUTO)
>> -			old_has_audio = intel_dp->has_audio;
>> -		else
>> -			old_has_audio = old_force_audio;
>> -
>> -		intel_dp->force_audio = i;
>> -
>> -		if (i == HDMI_AUDIO_AUTO)
>> -			has_audio = intel_dp->has_audio;
>> -		else
>> -			has_audio = (i == HDMI_AUDIO_ON);
>> -
>> -		if (has_audio == old_has_audio)
>> -			return 0;
>> -
>> -		goto done;
>> -	}
>> -
>> -	if (property == dev_priv->broadcast_rgb_property) {
>> -		bool old_auto = intel_dp->color_range_auto;
>> -		bool old_range = intel_dp->limited_color_range;
>> -
>> -		switch (val) {
>> -		case INTEL_BROADCAST_RGB_AUTO:
>> -			intel_dp->color_range_auto = true;
>> -			break;
>> -		case INTEL_BROADCAST_RGB_FULL:
>> -			intel_dp->color_range_auto = false;
>> -			intel_dp->limited_color_range = false;
>> -			break;
>> -		case INTEL_BROADCAST_RGB_LIMITED:
>> -			intel_dp->color_range_auto = false;
>> -			intel_dp->limited_color_range = true;
>> -			break;
>> -		default:
>> -			return -EINVAL;
>> -		}
>> -
>> -		if (old_auto == intel_dp->color_range_auto &&
>> -		    old_range == intel_dp->limited_color_range)
>> -			return 0;
>> -
>> -		goto done;
>> -	}
>> -
>> -	if (is_edp(intel_dp) &&
>> -	    property == connector->dev->mode_config.scaling_mode_property) {
>> -		if (val == DRM_MODE_SCALE_NONE) {
>> -			DRM_DEBUG_KMS("no scaling not supported\n");
>> -			return -EINVAL;
>> -		}
>> -		if (HAS_GMCH_DISPLAY(dev_priv) &&
>> -		    val == DRM_MODE_SCALE_CENTER) {
>> -			DRM_DEBUG_KMS("centering not supported\n");
>> -			return -EINVAL;
>> -		}
>> -
>> -		if (intel_connector->panel.fitting_mode == val) {
>> -			/* the eDP scaling property is not changed */
>> -			return 0;
>> -		}
>> -		intel_connector->panel.fitting_mode = val;
>> -
>> -		goto done;
>> -	}
>> -
>> -	return -EINVAL;
>> -
>> -done:
>> -	if (intel_encoder->base.crtc)
>> -		intel_crtc_restore_mode(intel_encoder->base.crtc);
>> -
>> -	return 0;
>> -}
>> -
>> -static int
>>  intel_dp_connector_register(struct drm_connector *connector)
>>  {
>>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> @@ -5036,19 +4940,21 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
>>  	.dpms = drm_atomic_helper_connector_dpms,
>>  	.force = intel_dp_force,
>>  	.fill_modes = drm_helper_probe_single_connector_modes,
>> -	.set_property = intel_dp_set_property,
>> -	.atomic_get_property = intel_connector_atomic_get_property,
>> +	.set_property = drm_atomic_helper_connector_set_property,
>> +	.atomic_get_property = intel_digital_connector_atomic_get_property,
>> +	.atomic_set_property = intel_digital_connector_atomic_set_property,
>>  	.late_register = intel_dp_connector_register,
>>  	.early_unregister = intel_dp_connector_unregister,
>>  	.destroy = intel_dp_connector_destroy,
>>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>>  };
>>  
>>  static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
>>  	.detect_ctx = intel_dp_detect,
>>  	.get_modes = intel_dp_get_modes,
>>  	.mode_valid = intel_dp_mode_valid,
>> +	.atomic_check = intel_digital_connector_atomic_check,
>>  };
>>  
>>  static const struct drm_encoder_funcs intel_dp_enc_funcs = {
>> @@ -5142,11 +5048,8 @@ bool intel_dp_is_edp(struct drm_i915_private *dev_priv, enum port port)
>>  static void
>>  intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector)
>>  {
>> -	struct intel_connector *intel_connector = to_intel_connector(connector);
>> -
>>  	intel_attach_force_audio_property(connector);
>>  	intel_attach_broadcast_rgb_property(connector);
>> -	intel_dp->color_range_auto = true;
>>  
>>  	if (is_edp(intel_dp)) {
>>  		drm_mode_create_scaling_mode_property(connector->dev);
>> @@ -5154,7 +5057,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>>  			&connector->base,
>>  			connector->dev->mode_config.scaling_mode_property,
>>  			DRM_MODE_SCALE_ASPECT);
>> -		intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
>> +
>> +		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>>  	}
>>  }
>>  
>> @@ -5797,6 +5701,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  	struct drm_display_mode *scan;
>>  	struct edid *edid;
>>  	enum pipe pipe = INVALID_PIPE;
>> +	unsigned allowed_fittings;
>>  
>>  	if (!is_edp(intel_dp))
>>  		return true;
>> @@ -5890,7 +5795,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  			      pipe_name(pipe));
>>  	}
>>  
>> -	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
>> +	allowed_fittings = 0;
>> +	if (!HAS_GMCH_DISPLAY(dev_priv))
>> +		allowed_fittings |= BIT(DRM_MODE_SCALE_CENTER);
>> +	allowed_fittings |= BIT(DRM_MODE_SCALE_ASPECT);
>> +	allowed_fittings |= BIT(DRM_MODE_SCALE_FULLSCREEN);
>> +
>> +	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode,
>> +			 allowed_fittings);
>>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>>  	intel_panel_setup_backlight(connector, pipe);
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 4df9e4531edc..9f0f80203472 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -261,7 +261,6 @@ struct intel_encoder {
>>  struct intel_panel {
>>  	struct drm_display_mode *fixed_mode;
>>  	struct drm_display_mode *downclock_mode;
>> -	int fitting_mode;
>>  	unsigned allowed_fitting_mode_mask;
>>  
>>  	/* backlight */
>> @@ -952,9 +951,7 @@ struct intel_dp {
>>  	bool detect_done;
>>  	bool channel_eq_status;
>>  	bool reset_link_params;
>> -	enum hdmi_force_audio force_audio;
>>  	bool limited_color_range;
> Can't we remove limited_color_range now too? Otherwise lgtm.
Indeed, thought it was used similar to intel_dp->has_audio detection but looks unused, will fix this.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 7/9] drm/i915: Convert intel_hdmi connector properties to atomic

[Maarten Lankhorst]

intel_hdmi supports 3 properties, force_audio, broadcast rgb and
scaling mode. The last one is only created for eDP, so the is_eDP
in set_property is not required.

panel fitting and broadcast rgb are straightforward and only requires
changing compute_config.

force_audio is also used to force DVI mode, which means changes to
compute_config and mode_valid. mode_valid is called with
connection_mutex held, so it can safely dereference connector->state.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |   4 -
 drivers/gpu/drm/i915/intel_hdmi.c | 164 ++++++++------------------------------
 2 files changed, 34 insertions(+), 134 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9f0f80203472..d1bba4790eae 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -872,13 +872,9 @@ struct intel_hdmi {
 		enum drm_dp_dual_mode_type type;
 		int max_tmds_clock;
 	} dp_dual_mode;
-	bool limited_color_range;
-	bool color_range_auto;
 	bool has_hdmi_sink;
 	bool has_audio;
-	enum hdmi_force_audio force_audio;
 	bool rgb_quant_range_selectable;
-	enum hdmi_picture_aspect aspect_ratio;
 	struct intel_connector *attached_connector;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 6efc3cb8c471..20368dcd471a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1218,7 +1218,8 @@ static int intel_hdmi_source_max_tmds_clock(struct drm_i915_private *dev_priv)
 }
 
 static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
-				 bool respect_downstream_limits)
+				 bool respect_downstream_limits,
+				 bool force_dvi)
 {
 	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
 	int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
@@ -1234,7 +1235,7 @@ static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
 		if (info->max_tmds_clock)
 			max_tmds_clock = min(max_tmds_clock,
 					     info->max_tmds_clock);
-		else if (!hdmi->has_hdmi_sink)
+		else if (!hdmi->has_hdmi_sink || force_dvi)
 			max_tmds_clock = min(max_tmds_clock, 165000);
 	}
 
@@ -1243,13 +1244,14 @@ static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
 
 static enum drm_mode_status
 hdmi_port_clock_valid(struct intel_hdmi *hdmi,
-		      int clock, bool respect_downstream_limits)
+		      int clock, bool respect_downstream_limits,
+		      bool force_dvi)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_hdmi_to_dev(hdmi));
 
 	if (clock < 25000)
 		return MODE_CLOCK_LOW;
-	if (clock > hdmi_port_clock_limit(hdmi, respect_downstream_limits))
+	if (clock > hdmi_port_clock_limit(hdmi, respect_downstream_limits, force_dvi))
 		return MODE_CLOCK_HIGH;
 
 	/* BXT DPLL can't generate 223-240 MHz */
@@ -1273,6 +1275,8 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	enum drm_mode_status status;
 	int clock;
 	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
+	bool force_dvi =
+		READ_ONCE(to_intel_digital_connector_state(connector->state)->force_audio) == HDMI_AUDIO_OFF_DVI;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
@@ -1289,11 +1293,11 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 		clock *= 2;
 
 	/* check if we can do 8bpc */
-	status = hdmi_port_clock_valid(hdmi, clock, true);
+	status = hdmi_port_clock_valid(hdmi, clock, true, force_dvi);
 
 	/* if we can't do 8bpc we may still be able to do 12bpc */
-	if (!HAS_GMCH_DISPLAY(dev_priv) && status != MODE_OK)
-		status = hdmi_port_clock_valid(hdmi, clock * 3 / 2, true);
+	if (!HAS_GMCH_DISPLAY(dev_priv) && status != MODE_OK && hdmi->has_hdmi_sink && !force_dvi)
+		status = hdmi_port_clock_valid(hdmi, clock * 3 / 2, true, force_dvi);
 
 	return status;
 }
@@ -1338,16 +1342,19 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc;
+	struct intel_digital_connector_state *intel_conn_state =
+		to_intel_digital_connector_state(conn_state);
 	int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
 	int clock_12bpc = clock_8bpc * 3 / 2;
 	int desired_bpp;
+	bool force_dvi = intel_conn_state->force_audio == HDMI_AUDIO_OFF_DVI;
 
-	pipe_config->has_hdmi_sink = intel_hdmi->has_hdmi_sink;
+	pipe_config->has_hdmi_sink = !force_dvi && intel_hdmi->has_hdmi_sink;
 
 	if (pipe_config->has_hdmi_sink)
 		pipe_config->has_infoframe = true;
 
-	if (intel_hdmi->color_range_auto) {
+	if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
 		pipe_config->limited_color_range =
 			pipe_config->has_hdmi_sink &&
@@ -1355,7 +1362,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			HDMI_QUANTIZATION_RANGE_LIMITED;
 	} else {
 		pipe_config->limited_color_range =
-			intel_hdmi->limited_color_range;
+			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) {
@@ -1367,8 +1374,13 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
-	if (pipe_config->has_hdmi_sink && intel_hdmi->has_audio)
-		pipe_config->has_audio = true;
+	if (pipe_config->has_hdmi_sink) {
+		if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
+			pipe_config->has_audio = intel_hdmi->has_audio;
+		else
+			pipe_config->has_audio =
+				intel_conn_state->force_audio == HDMI_AUDIO_ON;
+	}
 
 	/*
 	 * HDMI is either 12 or 8, so if the display lets 10bpc sneak
@@ -1376,8 +1388,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	 * outputs. We also need to check that the higher clock still fits
 	 * within limits.
 	 */
-	if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink &&
-	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, true) == MODE_OK &&
+	if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink && !force_dvi &&
+	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, true, force_dvi) == MODE_OK &&
 	    hdmi_12bpc_possible(pipe_config)) {
 		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
 		desired_bpp = 12*3;
@@ -1397,13 +1409,13 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	}
 
 	if (hdmi_port_clock_valid(intel_hdmi, pipe_config->port_clock,
-				  false) != MODE_OK) {
+				  false, force_dvi) != MODE_OK) {
 		DRM_DEBUG_KMS("unsupported HDMI clock, rejecting mode\n");
 		return false;
 	}
 
 	/* Set user selected PAR to incoming mode's member */
-	adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
+	adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio;
 
 	pipe_config->lane_count = 4;
 
@@ -1504,13 +1516,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
 			drm_rgb_quant_range_selectable(edid);
 
 		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
-		if (intel_hdmi->force_audio != HDMI_AUDIO_AUTO)
-			intel_hdmi->has_audio =
-				intel_hdmi->force_audio == HDMI_AUDIO_ON;
-
-		if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
-			intel_hdmi->has_hdmi_sink =
-				drm_detect_hdmi_monitor(edid);
+		intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
 
 		connected = true;
 	}
@@ -1572,108 +1578,6 @@ static int intel_hdmi_get_modes(struct drm_connector *connector)
 	return intel_connector_update_modes(connector, edid);
 }
 
-static bool
-intel_hdmi_detect_audio(struct drm_connector *connector)
-{
-	bool has_audio = false;
-	struct edid *edid;
-
-	edid = to_intel_connector(connector)->detect_edid;
-	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL)
-		has_audio = drm_detect_monitor_audio(edid);
-
-	return has_audio;
-}
-
-static int
-intel_hdmi_set_property(struct drm_connector *connector,
-			struct drm_property *property,
-			uint64_t val)
-{
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-	struct intel_digital_port *intel_dig_port =
-		hdmi_to_dig_port(intel_hdmi);
-	struct drm_i915_private *dev_priv = to_i915(connector->dev);
-	int ret;
-
-	ret = drm_object_property_set_value(&connector->base, property, val);
-	if (ret)
-		return ret;
-
-	if (property == dev_priv->force_audio_property) {
-		enum hdmi_force_audio i = val;
-		bool has_audio;
-
-		if (i == intel_hdmi->force_audio)
-			return 0;
-
-		intel_hdmi->force_audio = i;
-
-		if (i == HDMI_AUDIO_AUTO)
-			has_audio = intel_hdmi_detect_audio(connector);
-		else
-			has_audio = (i == HDMI_AUDIO_ON);
-
-		if (i == HDMI_AUDIO_OFF_DVI)
-			intel_hdmi->has_hdmi_sink = 0;
-
-		intel_hdmi->has_audio = has_audio;
-		goto done;
-	}
-
-	if (property == dev_priv->broadcast_rgb_property) {
-		bool old_auto = intel_hdmi->color_range_auto;
-		bool old_range = intel_hdmi->limited_color_range;
-
-		switch (val) {
-		case INTEL_BROADCAST_RGB_AUTO:
-			intel_hdmi->color_range_auto = true;
-			break;
-		case INTEL_BROADCAST_RGB_FULL:
-			intel_hdmi->color_range_auto = false;
-			intel_hdmi->limited_color_range = false;
-			break;
-		case INTEL_BROADCAST_RGB_LIMITED:
-			intel_hdmi->color_range_auto = false;
-			intel_hdmi->limited_color_range = true;
-			break;
-		default:
-			return -EINVAL;
-		}
-
-		if (old_auto == intel_hdmi->color_range_auto &&
-		    old_range == intel_hdmi->limited_color_range)
-			return 0;
-
-		goto done;
-	}
-
-	if (property == connector->dev->mode_config.aspect_ratio_property) {
-		switch (val) {
-		case DRM_MODE_PICTURE_ASPECT_NONE:
-			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
-			break;
-		case DRM_MODE_PICTURE_ASPECT_4_3:
-			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
-			break;
-		case DRM_MODE_PICTURE_ASPECT_16_9:
-			intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
-			break;
-		default:
-			return -EINVAL;
-		}
-		goto done;
-	}
-
-	return -EINVAL;
-
-done:
-	if (intel_dig_port->base.base.crtc)
-		intel_crtc_restore_mode(intel_dig_port->base.base.crtc);
-
-	return 0;
-}
-
 static void intel_hdmi_pre_enable(struct intel_encoder *encoder,
 				  struct intel_crtc_state *pipe_config,
 				  struct drm_connector_state *conn_state)
@@ -1798,18 +1702,20 @@ static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
 	.detect = intel_hdmi_detect,
 	.force = intel_hdmi_force,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = intel_hdmi_set_property,
-	.atomic_get_property = intel_connector_atomic_get_property,
+	.set_property = drm_atomic_helper_connector_set_property,
+	.atomic_get_property = intel_digital_connector_atomic_get_property,
+	.atomic_set_property = intel_digital_connector_atomic_set_property,
 	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_hdmi_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
 };
 
 static const struct drm_connector_helper_funcs intel_hdmi_connector_helper_funcs = {
 	.get_modes = intel_hdmi_get_modes,
 	.mode_valid = intel_hdmi_mode_valid,
+	.atomic_check = intel_digital_connector_atomic_check,
 };
 
 static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
@@ -1821,9 +1727,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 {
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
-	intel_hdmi->color_range_auto = true;
 	intel_attach_aspect_ratio_property(connector);
-	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
 /*
-- 
2.7.4

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

[PATCH v4 8/9] drm/i915: Handle force_audio correctly in intel_sdvo

[Maarten Lankhorst]

Do the same as other connectors, attempt to detect hdmi audio in
the detect() callback, and only use the force_audio property as
override. Compute has_audio in pipe_config, and use that value
instead of the probed value directly.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 51 ++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 816a6f5a3fd9..9855e4d48542 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1127,6 +1127,8 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 				      struct drm_connector_state *conn_state)
 {
 	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
+	struct intel_sdvo_connector *intel_sdvo_connector =
+		to_intel_sdvo_connector(conn_state->connector);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct drm_display_mode *mode = &pipe_config->base.mode;
 
@@ -1165,7 +1167,12 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 	pipe_config->pixel_multiplier =
 		intel_sdvo_get_pixel_multiplier(adjusted_mode);
 
-	pipe_config->has_hdmi_sink = intel_sdvo->has_hdmi_monitor;
+	if (intel_sdvo_connector->force_audio != HDMI_AUDIO_OFF_DVI)
+		pipe_config->has_hdmi_sink = intel_sdvo->has_hdmi_monitor;
+
+	if (intel_sdvo_connector->force_audio == HDMI_AUDIO_ON ||
+	    (intel_sdvo_connector->force_audio == HDMI_AUDIO_AUTO && intel_sdvo->has_hdmi_audio))
+		pipe_config->has_audio = true;
 
 	if (intel_sdvo->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
@@ -1290,7 +1297,7 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
 	else
 		sdvox |= SDVO_PIPE_SEL(crtc->pipe);
 
-	if (intel_sdvo->has_hdmi_audio)
+	if (crtc_state->has_audio)
 		sdvox |= SDVO_AUDIO_ENABLE;
 
 	if (INTEL_GEN(dev_priv) >= 4) {
@@ -1699,12 +1706,6 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
 		kfree(edid);
 	}
 
-	if (status == connector_status_connected) {
-		struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
-		if (intel_sdvo_connector->force_audio != HDMI_AUDIO_AUTO)
-			intel_sdvo->has_hdmi_audio = (intel_sdvo_connector->force_audio == HDMI_AUDIO_ON);
-	}
-
 	return status;
 }
 
@@ -1983,23 +1984,6 @@ static void intel_sdvo_destroy(struct drm_connector *connector)
 	kfree(intel_sdvo_connector);
 }
 
-static bool intel_sdvo_detect_hdmi_audio(struct drm_connector *connector)
-{
-	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
-	struct edid *edid;
-	bool has_audio = false;
-
-	if (!intel_sdvo->is_hdmi)
-		return false;
-
-	edid = intel_sdvo_get_edid(connector);
-	if (edid != NULL && edid->input & DRM_EDID_INPUT_DIGITAL)
-		has_audio = drm_detect_monitor_audio(edid);
-	kfree(edid);
-
-	return has_audio;
-}
-
 static int
 intel_sdvo_set_property(struct drm_connector *connector,
 			struct drm_property *property,
@@ -2018,22 +2002,23 @@ intel_sdvo_set_property(struct drm_connector *connector,
 
 	if (property == dev_priv->force_audio_property) {
 		int i = val;
-		bool has_audio;
-
-		if (i == intel_sdvo_connector->force_audio)
-			return 0;
+		bool has_audio, old_audio;
 
-		intel_sdvo_connector->force_audio = i;
+		if (intel_sdvo_connector->force_audio == HDMI_AUDIO_AUTO)
+			old_audio = intel_sdvo->has_hdmi_audio;
+		else
+			old_audio = intel_sdvo_connector->force_audio == HDMI_AUDIO_ON;
 
 		if (i == HDMI_AUDIO_AUTO)
-			has_audio = intel_sdvo_detect_hdmi_audio(connector);
+			has_audio = intel_sdvo->has_hdmi_audio;
 		else
 			has_audio = (i == HDMI_AUDIO_ON);
 
-		if (has_audio == intel_sdvo->has_hdmi_audio)
+		intel_sdvo_connector->force_audio = i;
+
+		if (has_audio == old_audio)
 			return 0;
 
-		intel_sdvo->has_hdmi_audio = has_audio;
 		goto done;
 	}
 
-- 
2.7.4

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

[PATCH v4 9/9] drm/i915: Convert intel_sdvo connector properties to atomic.

[Maarten Lankhorst]

SDVO was the last connector that's still using the legacy paths
for properties, and this is with a reason!

This connector implements a lot of properties dynamically,
and some of them shared with the digital connector state,
so sdvo_connector_state subclasses intel_digital_connector_state.

set_property had a lot of validation, but this is handled in the
drm core, so most of the validation can die off. The properties
are written right before enabling the connector, since there is no
good way to update the properties without crtc.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c  |  40 ---
 drivers/gpu/drm/i915/intel_display.c |  37 ---
 drivers/gpu/drm/i915/intel_drv.h     |   6 -
 drivers/gpu/drm/i915/intel_sdvo.c    | 538 ++++++++++++++++++-----------------
 4 files changed, 281 insertions(+), 340 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index eb72166675f0..f068b623cf10 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -36,46 +36,6 @@
 #include "intel_drv.h"
 
 /**
- * intel_connector_atomic_get_property - fetch legacy connector property value
- * @connector: connector to fetch property for
- * @state: state containing the property value
- * @property: property to look up
- * @val: pointer to write property value into
- *
- * The DRM core does not store shadow copies of properties for
- * atomic-capable drivers.  This entrypoint is used to fetch
- * the current value of a driver-specific connector property.
- *
- * This is a intermediary solution until all connectors are
- * converted to support full atomic properties.
- */
-int intel_connector_atomic_get_property(struct drm_connector *connector,
-					const struct drm_connector_state *state,
-					struct drm_property *property,
-					uint64_t *val)
-{
-	int i;
-
-	/*
-	 * TODO: We only have atomic modeset for planes at the moment, so the
-	 * crtc/connector code isn't quite ready yet.  Until it's ready,
-	 * continue to look up all property values in the DRM's shadow copy
-	 * in obj->properties->values[].
-	 *
-	 * When the crtc/connector state work matures, this function should
-	 * be updated to read the values out of the state structure instead.
-	 */
-	for (i = 0; i < connector->base.properties->count; i++) {
-		if (connector->base.properties->properties[i] == property) {
-			*val = connector->base.properties->values[i];
-			return 0;
-		}
-	}
-
-	return -EINVAL;
-}
-
-/**
  * intel_digital_connector_atomic_get_property - hook for connector->atomic_get_property.
  * @connector: Connector to get the property for.
  * @state: Connector state to retrieve the property from.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5eb4ddb04797..c2c367b90c0a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13075,43 +13075,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 	return 0;
 }
 
-void intel_crtc_restore_mode(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_atomic_state *state;
-	struct drm_crtc_state *crtc_state;
-	int ret;
-
-	state = drm_atomic_state_alloc(dev);
-	if (!state) {
-		DRM_DEBUG_KMS("[CRTC:%d:%s] crtc restore failed, out of memory",
-			      crtc->base.id, crtc->name);
-		return;
-	}
-
-	state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;
-
-retry:
-	crtc_state = drm_atomic_get_crtc_state(state, crtc);
-	ret = PTR_ERR_OR_ZERO(crtc_state);
-	if (!ret) {
-		if (!crtc_state->active)
-			goto out;
-
-		crtc_state->mode_changed = true;
-		ret = drm_atomic_commit(state);
-	}
-
-	if (ret == -EDEADLK) {
-		drm_atomic_state_clear(state);
-		drm_modeset_backoff(state->acquire_ctx);
-		goto retry;
-	}
-
-out:
-	drm_atomic_state_put(state);
-}
-
 static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.set_config = drm_atomic_helper_set_config,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d1bba4790eae..d2118413dd18 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1318,7 +1318,6 @@ unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info
 bool intel_has_pending_fb_unpin(struct drm_i915_private *dev_priv);
 void intel_mark_busy(struct drm_i915_private *dev_priv);
 void intel_mark_idle(struct drm_i915_private *dev_priv);
-void intel_crtc_restore_mode(struct drm_crtc *crtc);
 int intel_display_suspend(struct drm_device *dev);
 void intel_pps_unlock_regs_wa(struct drm_i915_private *dev_priv);
 void intel_encoder_destroy(struct drm_encoder *encoder);
@@ -1879,11 +1878,6 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
 void intel_tv_init(struct drm_i915_private *dev_priv);
 
 /* intel_atomic.c */
-int intel_connector_atomic_get_property(struct drm_connector *connector,
-					const struct drm_connector_state *state,
-					struct drm_property *property,
-					uint64_t *val);
-
 int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
 						const struct drm_connector_state *state,
 						struct drm_property *property,
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 9855e4d48542..0aac7ce2b218 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -100,18 +100,6 @@ struct intel_sdvo {
 	uint16_t hotplug_active;
 
 	/**
-	 * This is used to select the color range of RBG outputs in HDMI mode.
-	 * It is only valid when using TMDS encoding and 8 bit per color mode.
-	 */
-	uint32_t color_range;
-	bool color_range_auto;
-
-	/**
-	 * HDMI user specified aspect ratio
-	 */
-	enum hdmi_picture_aspect aspect_ratio;
-
-	/**
 	 * This is set if we're going to treat the device as TV-out.
 	 *
 	 * While we have these nice friendly flags for output types that ought
@@ -122,9 +110,6 @@ struct intel_sdvo {
 
 	enum port port;
 
-	/* This is for current tv format name */
-	int tv_format_index;
-
 	/**
 	 * This is set if we treat the device as HDMI, instead of DVI.
 	 */
@@ -159,8 +144,6 @@ struct intel_sdvo_connector {
 	/* Mark the type of connector */
 	uint16_t output_flag;
 
-	enum hdmi_force_audio force_audio;
-
 	/* This contains all current supported TV format */
 	u8 tv_format_supported[TV_FORMAT_NUM];
 	int   format_supported_num;
@@ -187,24 +170,19 @@ struct intel_sdvo_connector {
 	/* add the property for the SDVO-TV/LVDS */
 	struct drm_property *brightness;
 
-	/* Add variable to record current setting for the above property */
-	u32	left_margin, right_margin, top_margin, bottom_margin;
-
 	/* this is to get the range of margin.*/
-	u32	max_hscan,  max_vscan;
-	u32	max_hpos, cur_hpos;
-	u32	max_vpos, cur_vpos;
-	u32	cur_brightness, max_brightness;
-	u32	cur_contrast,	max_contrast;
-	u32	cur_saturation, max_saturation;
-	u32	cur_hue,	max_hue;
-	u32	cur_sharpness,	max_sharpness;
-	u32	cur_flicker_filter,		max_flicker_filter;
-	u32	cur_flicker_filter_adaptive,	max_flicker_filter_adaptive;
-	u32	cur_flicker_filter_2d,		max_flicker_filter_2d;
-	u32	cur_tv_chroma_filter,	max_tv_chroma_filter;
-	u32	cur_tv_luma_filter,	max_tv_luma_filter;
-	u32	cur_dot_crawl,	max_dot_crawl;
+	u32 max_hscan, max_vscan;
+};
+
+struct intel_sdvo_connector_state {
+	/* base.base: tv.saturation/contrast/hue/brightness */
+	struct intel_digital_connector_state base;
+
+	struct {
+		unsigned overscan_h, overscan_v, hpos, vpos, sharpness;
+		unsigned flicker_filter, flicker_filter_2d, flicker_filter_adaptive;
+		unsigned chroma_filter, luma_filter, dot_crawl;
+	} tv;
 };
 
 static struct intel_sdvo *to_sdvo(struct intel_encoder *encoder)
@@ -217,9 +195,16 @@ static struct intel_sdvo *intel_attached_sdvo(struct drm_connector *connector)
 	return to_sdvo(intel_attached_encoder(connector));
 }
 
-static struct intel_sdvo_connector *to_intel_sdvo_connector(struct drm_connector *connector)
+static struct intel_sdvo_connector *
+to_intel_sdvo_connector(struct drm_connector *connector)
 {
-	return container_of(to_intel_connector(connector), struct intel_sdvo_connector, base);
+	return container_of(connector, struct intel_sdvo_connector, base.base);
+}
+
+static struct intel_sdvo_connector_state *
+to_intel_sdvo_connector_state(struct drm_connector_state *conn_state)
+{
+	return container_of(conn_state, struct intel_sdvo_connector_state, base.base);
 }
 
 static bool
@@ -1035,12 +1020,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 					  sdvo_data, sizeof(sdvo_data));
 }
 
-static bool intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo)
+static bool intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo,
+				     struct drm_connector_state *conn_state)
 {
 	struct intel_sdvo_tv_format format;
 	uint32_t format_map;
 
-	format_map = 1 << intel_sdvo->tv_format_index;
+	format_map = 1 << conn_state->tv.mode;
 	memset(&format, 0, sizeof(format));
 	memcpy(&format, &format_map, min(sizeof(format), sizeof(format_map)));
 
@@ -1127,8 +1113,8 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 				      struct drm_connector_state *conn_state)
 {
 	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
-	struct intel_sdvo_connector *intel_sdvo_connector =
-		to_intel_sdvo_connector(conn_state->connector);
+	struct intel_sdvo_connector_state *intel_sdvo_state =
+		to_intel_sdvo_connector_state(conn_state);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct drm_display_mode *mode = &pipe_config->base.mode;
 
@@ -1167,14 +1153,14 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 	pipe_config->pixel_multiplier =
 		intel_sdvo_get_pixel_multiplier(adjusted_mode);
 
-	if (intel_sdvo_connector->force_audio != HDMI_AUDIO_OFF_DVI)
+	if (intel_sdvo_state->base.force_audio != HDMI_AUDIO_OFF_DVI)
 		pipe_config->has_hdmi_sink = intel_sdvo->has_hdmi_monitor;
 
-	if (intel_sdvo_connector->force_audio == HDMI_AUDIO_ON ||
-	    (intel_sdvo_connector->force_audio == HDMI_AUDIO_AUTO && intel_sdvo->has_hdmi_audio))
+	if (intel_sdvo_state->base.force_audio == HDMI_AUDIO_ON ||
+	    (intel_sdvo_state->base.force_audio == HDMI_AUDIO_AUTO && intel_sdvo->has_hdmi_audio))
 		pipe_config->has_audio = true;
 
-	if (intel_sdvo->color_range_auto) {
+	if (intel_sdvo_state->base.broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
 		/* FIXME: This bit is only valid when using TMDS encoding and 8
 		 * bit per color mode. */
@@ -1183,7 +1169,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 			pipe_config->limited_color_range = true;
 	} else {
 		if (pipe_config->has_hdmi_sink &&
-		    intel_sdvo->color_range == HDMI_COLOR_RANGE_16_235)
+		    intel_sdvo_state->base.broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED)
 			pipe_config->limited_color_range = true;
 	}
 
@@ -1193,11 +1179,73 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 
 	/* Set user selected PAR to incoming mode's member */
 	if (intel_sdvo->is_hdmi)
-		adjusted_mode->picture_aspect_ratio = intel_sdvo->aspect_ratio;
+		adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio;
 
 	return true;
 }
 
+#define UPDATE_PROPERTY(input, NAME) \
+	do { \
+		val = input; \
+		intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_##NAME, &val, sizeof(val)); \
+	} while (0)
+
+static void intel_sdvo_update_props(struct intel_sdvo *intel_sdvo,
+				    struct intel_sdvo_connector_state *sdvo_state)
+{
+	struct drm_connector_state *conn_state = &sdvo_state->base.base;
+	struct intel_sdvo_connector *intel_sdvo_conn =
+		to_intel_sdvo_connector(conn_state->connector);
+	uint16_t val;
+
+	if (intel_sdvo_conn->left)
+		UPDATE_PROPERTY(sdvo_state->tv.overscan_h, OVERSCAN_H);
+
+	if (intel_sdvo_conn->top)
+		UPDATE_PROPERTY(sdvo_state->tv.overscan_v, OVERSCAN_V);
+
+	if (intel_sdvo_conn->hpos)
+		UPDATE_PROPERTY(sdvo_state->tv.hpos, HPOS);
+
+	if (intel_sdvo_conn->vpos)
+		UPDATE_PROPERTY(sdvo_state->tv.vpos, VPOS);
+
+	if (intel_sdvo_conn->saturation)
+		UPDATE_PROPERTY(conn_state->tv.saturation, SATURATION);
+
+	if (intel_sdvo_conn->contrast)
+		UPDATE_PROPERTY(conn_state->tv.contrast, CONTRAST);
+
+	if (intel_sdvo_conn->hue)
+		UPDATE_PROPERTY(conn_state->tv.hue, HUE);
+
+	if (intel_sdvo_conn->brightness)
+		UPDATE_PROPERTY(conn_state->tv.brightness, BRIGHTNESS);
+
+	if (intel_sdvo_conn->sharpness)
+		UPDATE_PROPERTY(sdvo_state->tv.sharpness, SHARPNESS);
+
+	if (intel_sdvo_conn->flicker_filter)
+		UPDATE_PROPERTY(sdvo_state->tv.flicker_filter, FLICKER_FILTER);
+
+	if (intel_sdvo_conn->flicker_filter_2d)
+		UPDATE_PROPERTY(sdvo_state->tv.flicker_filter_2d, FLICKER_FILTER_2D);
+
+	if (intel_sdvo_conn->flicker_filter_adaptive)
+		UPDATE_PROPERTY(sdvo_state->tv.flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE);
+
+	if (intel_sdvo_conn->tv_chroma_filter)
+		UPDATE_PROPERTY(sdvo_state->tv.chroma_filter, TV_CHROMA_FILTER);
+
+	if (intel_sdvo_conn->tv_luma_filter)
+		UPDATE_PROPERTY(sdvo_state->tv.luma_filter, TV_LUMA_FILTER);
+
+	if (intel_sdvo_conn->dot_crawl)
+		UPDATE_PROPERTY(sdvo_state->tv.dot_crawl, DOT_CRAWL);
+
+#undef UPDATE_PROPERTY
+}
+
 static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
 				  struct intel_crtc_state *crtc_state,
 				  struct drm_connector_state *conn_state)
@@ -1205,6 +1253,7 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
 	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
+	struct intel_sdvo_connector_state *sdvo_state = to_intel_sdvo_connector_state(conn_state);
 	struct drm_display_mode *mode = &crtc_state->base.mode;
 	struct intel_sdvo *intel_sdvo = to_sdvo(intel_encoder);
 	u32 sdvox;
@@ -1212,6 +1261,8 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
 	struct intel_sdvo_dtd input_dtd, output_dtd;
 	int rate;
 
+	intel_sdvo_update_props(intel_sdvo, sdvo_state);
+
 	/* First, set the input mapping for the first input to our controlled
 	 * output. This is only correct if we're a single-input device, in
 	 * which case the first input is the output from the appropriate SDVO
@@ -1253,7 +1304,7 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
 		intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
 
 	if (intel_sdvo->is_tv &&
-	    !intel_sdvo_set_tv_format(intel_sdvo))
+	    !intel_sdvo_set_tv_format(intel_sdvo, conn_state))
 		return;
 
 	intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
@@ -1885,6 +1936,7 @@ static const struct drm_display_mode sdvo_tv_modes[] = {
 static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
 {
 	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
+	const struct drm_connector_state *conn_state = connector->state;
 	struct intel_sdvo_sdtv_resolution_request tv_res;
 	uint32_t reply = 0, format_map = 0;
 	int i;
@@ -1895,7 +1947,7 @@ static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
 	/* Read the list of supported input resolutions for the selected TV
 	 * format.
 	 */
-	format_map = 1 << intel_sdvo->tv_format_index;
+	format_map = 1 << conn_state->tv.mode;
 	memcpy(&tv_res, &format_map,
 	       min(sizeof(format_map), sizeof(struct intel_sdvo_sdtv_resolution_request)));
 
@@ -1985,187 +2037,120 @@ static void intel_sdvo_destroy(struct drm_connector *connector)
 }
 
 static int
-intel_sdvo_set_property(struct drm_connector *connector,
-			struct drm_property *property,
-			uint64_t val)
+intel_sdvo_connector_atomic_get_property(struct drm_connector *connector,
+					 const struct drm_connector_state *state,
+					 struct drm_property *property,
+					 uint64_t *val)
 {
-	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
 	struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
-	struct drm_i915_private *dev_priv = to_i915(connector->dev);
-	uint16_t temp_value;
-	uint8_t cmd;
-	int ret;
-
-	ret = drm_object_property_set_value(&connector->base, property, val);
-	if (ret)
-		return ret;
-
-	if (property == dev_priv->force_audio_property) {
-		int i = val;
-		bool has_audio, old_audio;
-
-		if (intel_sdvo_connector->force_audio == HDMI_AUDIO_AUTO)
-			old_audio = intel_sdvo->has_hdmi_audio;
-		else
-			old_audio = intel_sdvo_connector->force_audio == HDMI_AUDIO_ON;
-
-		if (i == HDMI_AUDIO_AUTO)
-			has_audio = intel_sdvo->has_hdmi_audio;
-		else
-			has_audio = (i == HDMI_AUDIO_ON);
-
-		intel_sdvo_connector->force_audio = i;
-
-		if (has_audio == old_audio)
-			return 0;
-
-		goto done;
-	}
-
-	if (property == dev_priv->broadcast_rgb_property) {
-		bool old_auto = intel_sdvo->color_range_auto;
-		uint32_t old_range = intel_sdvo->color_range;
-
-		switch (val) {
-		case INTEL_BROADCAST_RGB_AUTO:
-			intel_sdvo->color_range_auto = true;
-			break;
-		case INTEL_BROADCAST_RGB_FULL:
-			intel_sdvo->color_range_auto = false;
-			intel_sdvo->color_range = 0;
-			break;
-		case INTEL_BROADCAST_RGB_LIMITED:
-			intel_sdvo->color_range_auto = false;
-			/* FIXME: this bit is only valid when using TMDS
-			 * encoding and 8 bit per color mode. */
-			intel_sdvo->color_range = HDMI_COLOR_RANGE_16_235;
-			break;
-		default:
-			return -EINVAL;
-		}
-
-		if (old_auto == intel_sdvo->color_range_auto &&
-		    old_range == intel_sdvo->color_range)
-			return 0;
-
-		goto done;
-	}
-
-	if (property == connector->dev->mode_config.aspect_ratio_property) {
-		switch (val) {
-		case DRM_MODE_PICTURE_ASPECT_NONE:
-			intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
-			break;
-		case DRM_MODE_PICTURE_ASPECT_4_3:
-			intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
-			break;
-		case DRM_MODE_PICTURE_ASPECT_16_9:
-			intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
-			break;
-		default:
-			return -EINVAL;
-		}
-		goto done;
-	}
-
-#define CHECK_PROPERTY(name, NAME) \
-	if (intel_sdvo_connector->name == property) { \
-		if (intel_sdvo_connector->cur_##name == temp_value) return 0; \
-		if (intel_sdvo_connector->max_##name < temp_value) return -EINVAL; \
-		cmd = SDVO_CMD_SET_##NAME; \
-		intel_sdvo_connector->cur_##name = temp_value; \
-		goto set_value; \
-	}
+	const struct intel_sdvo_connector_state *sdvo_state = to_intel_sdvo_connector_state((void *)state);
 
 	if (property == intel_sdvo_connector->tv_format) {
-		if (val >= TV_FORMAT_NUM)
-			return -EINVAL;
-
-		if (intel_sdvo->tv_format_index ==
-		    intel_sdvo_connector->tv_format_supported[val])
-			return 0;
-
-		intel_sdvo->tv_format_index = intel_sdvo_connector->tv_format_supported[val];
-		goto done;
-	} else if (IS_TV_OR_LVDS(intel_sdvo_connector)) {
-		temp_value = val;
-		if (intel_sdvo_connector->left == property) {
-			drm_object_property_set_value(&connector->base,
-							 intel_sdvo_connector->right, val);
-			if (intel_sdvo_connector->left_margin == temp_value)
-				return 0;
+		int i;
 
-			intel_sdvo_connector->left_margin = temp_value;
-			intel_sdvo_connector->right_margin = temp_value;
-			temp_value = intel_sdvo_connector->max_hscan -
-				intel_sdvo_connector->left_margin;
-			cmd = SDVO_CMD_SET_OVERSCAN_H;
-			goto set_value;
-		} else if (intel_sdvo_connector->right == property) {
-			drm_object_property_set_value(&connector->base,
-							 intel_sdvo_connector->left, val);
-			if (intel_sdvo_connector->right_margin == temp_value)
-				return 0;
+		for (i = 0; i < intel_sdvo_connector->format_supported_num; i++)
+			if (state->tv.mode == intel_sdvo_connector->tv_format_supported[i]) {
+				*val = i;
 
-			intel_sdvo_connector->left_margin = temp_value;
-			intel_sdvo_connector->right_margin = temp_value;
-			temp_value = intel_sdvo_connector->max_hscan -
-				intel_sdvo_connector->left_margin;
-			cmd = SDVO_CMD_SET_OVERSCAN_H;
-			goto set_value;
-		} else if (intel_sdvo_connector->top == property) {
-			drm_object_property_set_value(&connector->base,
-							 intel_sdvo_connector->bottom, val);
-			if (intel_sdvo_connector->top_margin == temp_value)
 				return 0;
+			}
 
-			intel_sdvo_connector->top_margin = temp_value;
-			intel_sdvo_connector->bottom_margin = temp_value;
-			temp_value = intel_sdvo_connector->max_vscan -
-				intel_sdvo_connector->top_margin;
-			cmd = SDVO_CMD_SET_OVERSCAN_V;
-			goto set_value;
-		} else if (intel_sdvo_connector->bottom == property) {
-			drm_object_property_set_value(&connector->base,
-							 intel_sdvo_connector->top, val);
-			if (intel_sdvo_connector->bottom_margin == temp_value)
-				return 0;
+		WARN_ON(1);
+		*val = 0;
+	} else if (property == intel_sdvo_connector->top ||
+		   property == intel_sdvo_connector->bottom)
+		*val = intel_sdvo_connector->max_vscan - sdvo_state->tv.overscan_v;
+	else if (property == intel_sdvo_connector->left ||
+		 property == intel_sdvo_connector->right)
+		*val = intel_sdvo_connector->max_hscan - sdvo_state->tv.overscan_h;
+	else if (property == intel_sdvo_connector->hpos)
+		*val = sdvo_state->tv.hpos;
+	else if (property == intel_sdvo_connector->vpos)
+		*val = sdvo_state->tv.vpos;
+	else if (property == intel_sdvo_connector->saturation)
+		*val = state->tv.saturation;
+	else if (property == intel_sdvo_connector->contrast)
+		*val = state->tv.contrast;
+	else if (property == intel_sdvo_connector->hue)
+		*val = state->tv.hue;
+	else if (property == intel_sdvo_connector->brightness)
+		*val = state->tv.brightness;
+	else if (property == intel_sdvo_connector->sharpness)
+		*val = sdvo_state->tv.sharpness;
+	else if (property == intel_sdvo_connector->flicker_filter)
+		*val = sdvo_state->tv.flicker_filter;
+	else if (property == intel_sdvo_connector->flicker_filter_2d)
+		*val = sdvo_state->tv.flicker_filter_2d;
+	else if (property == intel_sdvo_connector->flicker_filter_adaptive)
+		*val = sdvo_state->tv.flicker_filter_adaptive;
+	else if (property == intel_sdvo_connector->tv_chroma_filter)
+		*val = sdvo_state->tv.chroma_filter;
+	else if (property == intel_sdvo_connector->tv_luma_filter)
+		*val = sdvo_state->tv.luma_filter;
+	else if (property == intel_sdvo_connector->dot_crawl)
+		*val = sdvo_state->tv.dot_crawl;
+	else
+		return intel_digital_connector_atomic_get_property(connector, state, property, val);
 
-			intel_sdvo_connector->top_margin = temp_value;
-			intel_sdvo_connector->bottom_margin = temp_value;
-			temp_value = intel_sdvo_connector->max_vscan -
-				intel_sdvo_connector->top_margin;
-			cmd = SDVO_CMD_SET_OVERSCAN_V;
-			goto set_value;
-		}
-		CHECK_PROPERTY(hpos, HPOS)
-		CHECK_PROPERTY(vpos, VPOS)
-		CHECK_PROPERTY(saturation, SATURATION)
-		CHECK_PROPERTY(contrast, CONTRAST)
-		CHECK_PROPERTY(hue, HUE)
-		CHECK_PROPERTY(brightness, BRIGHTNESS)
-		CHECK_PROPERTY(sharpness, SHARPNESS)
-		CHECK_PROPERTY(flicker_filter, FLICKER_FILTER)
-		CHECK_PROPERTY(flicker_filter_2d, FLICKER_FILTER_2D)
-		CHECK_PROPERTY(flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE)
-		CHECK_PROPERTY(tv_chroma_filter, TV_CHROMA_FILTER)
-		CHECK_PROPERTY(tv_luma_filter, TV_LUMA_FILTER)
-		CHECK_PROPERTY(dot_crawl, DOT_CRAWL)
-	}
+	return 0;
+}
 
-	return -EINVAL; /* unknown property */
+static int
+intel_sdvo_connector_atomic_set_property(struct drm_connector *connector,
+					 struct drm_connector_state *state,
+					 struct drm_property *property,
+					 uint64_t val)
+{
+	struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
+	struct intel_sdvo_connector_state *sdvo_state = to_intel_sdvo_connector_state(state);
 
-set_value:
-	if (!intel_sdvo_set_value(intel_sdvo, cmd, &temp_value, 2))
-		return -EIO;
+	if (property == intel_sdvo_connector->tv_format) {
+		state->tv.mode = intel_sdvo_connector->tv_format_supported[val];
 
+		if (state->crtc) {
+			struct drm_crtc_state *crtc_state =
+				drm_atomic_get_new_crtc_state(state->state, state->crtc);
 
-done:
-	if (intel_sdvo->base.base.crtc)
-		intel_crtc_restore_mode(intel_sdvo->base.base.crtc);
+			crtc_state->connectors_changed = true;
+		}
+	} else if (property == intel_sdvo_connector->top ||
+		   property == intel_sdvo_connector->bottom)
+		/* Cannot set these independent from each other */
+		sdvo_state->tv.overscan_v = intel_sdvo_connector->max_vscan - val;
+	else if (property == intel_sdvo_connector->left ||
+		 property == intel_sdvo_connector->right)
+		/* Cannot set these independent from each other */
+		sdvo_state->tv.overscan_h = intel_sdvo_connector->max_hscan - val;
+	else if (property == intel_sdvo_connector->hpos)
+		sdvo_state->tv.hpos = val;
+	else if (property == intel_sdvo_connector->vpos)
+		sdvo_state->tv.vpos = val;
+	else if (property == intel_sdvo_connector->saturation)
+		state->tv.saturation = val;
+	else if (property == intel_sdvo_connector->contrast)
+		state->tv.contrast = val;
+	else if (property == intel_sdvo_connector->hue)
+		state->tv.hue = val;
+	else if (property == intel_sdvo_connector->brightness)
+		state->tv.brightness = val;
+	else if (property == intel_sdvo_connector->sharpness)
+		sdvo_state->tv.sharpness = val;
+	else if (property == intel_sdvo_connector->flicker_filter)
+		sdvo_state->tv.flicker_filter = val;
+	else if (property == intel_sdvo_connector->flicker_filter_2d)
+		sdvo_state->tv.flicker_filter_2d = val;
+	else if (property == intel_sdvo_connector->flicker_filter_adaptive)
+		sdvo_state->tv.flicker_filter_adaptive = val;
+	else if (property == intel_sdvo_connector->tv_chroma_filter)
+		sdvo_state->tv.chroma_filter = val;
+	else if (property == intel_sdvo_connector->tv_luma_filter)
+		sdvo_state->tv.luma_filter = val;
+	else if (property == intel_sdvo_connector->dot_crawl)
+		sdvo_state->tv.dot_crawl = val;
+	else
+		return intel_digital_connector_atomic_set_property(connector, state, property, val);
 
 	return 0;
-#undef CHECK_PROPERTY
 }
 
 static int
@@ -2193,22 +2178,61 @@ intel_sdvo_connector_unregister(struct drm_connector *connector)
 	intel_connector_unregister(connector);
 }
 
+static struct drm_connector_state *
+intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
+{
+	struct intel_sdvo_connector_state *state;
+
+	state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	__drm_atomic_helper_connector_duplicate_state(connector, &state->base.base);
+	return &state->base.base;
+}
+
 static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_sdvo_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = intel_sdvo_set_property,
-	.atomic_get_property = intel_connector_atomic_get_property,
+	.set_property = drm_atomic_helper_connector_set_property,
+	.atomic_get_property = intel_sdvo_connector_atomic_get_property,
+	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
 	.late_register = intel_sdvo_connector_register,
 	.early_unregister = intel_sdvo_connector_unregister,
 	.destroy = intel_sdvo_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_duplicate_state = intel_sdvo_connector_duplicate_state,
 };
 
+static int intel_sdvo_atomic_check(struct drm_connector *conn,
+				   struct drm_connector_state *new_conn_state)
+{
+	struct drm_atomic_state *state = new_conn_state->state;
+	struct drm_connector_state *old_conn_state =
+		drm_atomic_get_old_connector_state(state, conn);
+	struct intel_sdvo_connector_state *old_state =
+		to_intel_sdvo_connector_state(old_conn_state);
+	struct intel_sdvo_connector_state *new_state =
+		to_intel_sdvo_connector_state(new_conn_state);
+
+	if (new_conn_state->crtc &&
+	    (memcmp(&old_state->tv, &new_state->tv, sizeof(old_state->tv)) ||
+	     memcmp(&old_conn_state->tv, &new_conn_state->tv, sizeof(old_conn_state->tv)))) {
+		struct drm_crtc_state *crtc_state =
+			drm_atomic_get_new_crtc_state(new_conn_state->state,
+						      new_conn_state->crtc);
+
+		crtc_state->connectors_changed = true;
+	}
+
+	return intel_digital_connector_atomic_check(conn, new_conn_state);
+}
+
 static const struct drm_connector_helper_funcs intel_sdvo_connector_helper_funcs = {
 	.get_modes = intel_sdvo_get_modes,
 	.mode_valid = intel_sdvo_mode_valid,
+	.atomic_check = intel_sdvo_atomic_check,
 };
 
 static void intel_sdvo_enc_destroy(struct drm_encoder *encoder)
@@ -2400,25 +2424,28 @@ intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo,
 	intel_attach_force_audio_property(&connector->base.base);
 	if (INTEL_GEN(dev_priv) >= 4 && IS_MOBILE(dev_priv)) {
 		intel_attach_broadcast_rgb_property(&connector->base.base);
-		intel_sdvo->color_range_auto = true;
 	}
 	intel_attach_aspect_ratio_property(&connector->base.base);
-	intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
 static struct intel_sdvo_connector *intel_sdvo_connector_alloc(void)
 {
 	struct intel_sdvo_connector *sdvo_connector;
+	struct intel_sdvo_connector_state *conn_state;
 
 	sdvo_connector = kzalloc(sizeof(*sdvo_connector), GFP_KERNEL);
 	if (!sdvo_connector)
 		return NULL;
 
-	if (intel_connector_init(&sdvo_connector->base) < 0) {
+	conn_state = kzalloc(sizeof(*conn_state), GFP_KERNEL);
+	if (!conn_state) {
 		kfree(sdvo_connector);
 		return NULL;
 	}
 
+	__drm_atomic_helper_connector_reset(&sdvo_connector->base.base,
+					    &conn_state->base.base);
+
 	return sdvo_connector;
 }
 
@@ -2710,31 +2737,30 @@ static bool intel_sdvo_tv_create_property(struct intel_sdvo *intel_sdvo,
 				intel_sdvo_connector->tv_format, i,
 				i, tv_format_names[intel_sdvo_connector->tv_format_supported[i]]);
 
-	intel_sdvo->tv_format_index = intel_sdvo_connector->tv_format_supported[0];
-	drm_object_attach_property(&intel_sdvo_connector->base.base.base,
-				      intel_sdvo_connector->tv_format, 0);
+	intel_sdvo_connector->base.base.state->tv.mode = intel_sdvo_connector->tv_format_supported[0];
+	drm_object_attach_property(&intel_sdvo_connector->base.base.base, 0, 0);
 	return true;
 
 }
 
-#define ENHANCEMENT(name, NAME) do { \
+#define _ENHANCEMENT(state_assignment, name, NAME) do { \
 	if (enhancements.name) { \
 		if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_MAX_##NAME, &data_value, 4) || \
 		    !intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_##NAME, &response, 2)) \
 			return false; \
-		intel_sdvo_connector->max_##name = data_value[0]; \
-		intel_sdvo_connector->cur_##name = response; \
 		intel_sdvo_connector->name = \
 			drm_property_create_range(dev, 0, #name, 0, data_value[0]); \
 		if (!intel_sdvo_connector->name) return false; \
+		state_assignment = response; \
 		drm_object_attach_property(&connector->base, \
-					      intel_sdvo_connector->name, \
-					      intel_sdvo_connector->cur_##name); \
+					   intel_sdvo_connector->name, 0); \
 		DRM_DEBUG_KMS(#name ": max %d, default %d, current %d\n", \
 			      data_value[0], data_value[1], response); \
 	} \
 } while (0)
 
+#define ENHANCEMENT(state, name, NAME) _ENHANCEMENT((state)->name, name, NAME)
+
 static bool
 intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
 				      struct intel_sdvo_connector *intel_sdvo_connector,
@@ -2742,6 +2768,9 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
 {
 	struct drm_device *dev = intel_sdvo->base.base.dev;
 	struct drm_connector *connector = &intel_sdvo_connector->base.base;
+	struct drm_connector_state *conn_state = connector->state;
+	struct intel_sdvo_connector_state *sdvo_state =
+		to_intel_sdvo_connector_state(conn_state);
 	uint16_t response, data_value[2];
 
 	/* when horizontal overscan is supported, Add the left/right  property */
@@ -2756,17 +2785,16 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
 					  &response, 2))
 			return false;
 
+		sdvo_state->tv.overscan_h = response;
+
 		intel_sdvo_connector->max_hscan = data_value[0];
-		intel_sdvo_connector->left_margin = data_value[0] - response;
-		intel_sdvo_connector->right_margin = intel_sdvo_connector->left_margin;
 		intel_sdvo_connector->left =
 			drm_property_create_range(dev, 0, "left_margin", 0, data_value[0]);
 		if (!intel_sdvo_connector->left)
 			return false;
 
 		drm_object_attach_property(&connector->base,
-					      intel_sdvo_connector->left,
-					      intel_sdvo_connector->left_margin);
+					   intel_sdvo_connector->left, 0);
 
 		intel_sdvo_connector->right =
 			drm_property_create_range(dev, 0, "right_margin", 0, data_value[0]);
@@ -2774,8 +2802,7 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
 			return false;
 
 		drm_object_attach_property(&connector->base,
-					      intel_sdvo_connector->right,
-					      intel_sdvo_connector->right_margin);
+					      intel_sdvo_connector->right, 0);
 		DRM_DEBUG_KMS("h_overscan: max %d, "
 			      "default %d, current %d\n",
 			      data_value[0], data_value[1], response);
@@ -2792,9 +2819,9 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
 					  &response, 2))
 			return false;
 
+		sdvo_state->tv.overscan_v = response;
+
 		intel_sdvo_connector->max_vscan = data_value[0];
-		intel_sdvo_connector->top_margin = data_value[0] - response;
-		intel_sdvo_connector->bottom_margin = intel_sdvo_connector->top_margin;
 		intel_sdvo_connector->top =
 			drm_property_create_range(dev, 0,
 					    "top_margin", 0, data_value[0]);
@@ -2802,8 +2829,7 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
 			return false;
 
 		drm_object_attach_property(&connector->base,
-					      intel_sdvo_connector->top,
-					      intel_sdvo_connector->top_margin);
+					   intel_sdvo_connector->top, 0);
 
 		intel_sdvo_connector->bottom =
 			drm_property_create_range(dev, 0,
@@ -2812,40 +2838,37 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
 			return false;
 
 		drm_object_attach_property(&connector->base,
-					      intel_sdvo_connector->bottom,
-					      intel_sdvo_connector->bottom_margin);
+					      intel_sdvo_connector->bottom, 0);
 		DRM_DEBUG_KMS("v_overscan: max %d, "
 			      "default %d, current %d\n",
 			      data_value[0], data_value[1], response);
 	}
 
-	ENHANCEMENT(hpos, HPOS);
-	ENHANCEMENT(vpos, VPOS);
-	ENHANCEMENT(saturation, SATURATION);
-	ENHANCEMENT(contrast, CONTRAST);
-	ENHANCEMENT(hue, HUE);
-	ENHANCEMENT(sharpness, SHARPNESS);
-	ENHANCEMENT(brightness, BRIGHTNESS);
-	ENHANCEMENT(flicker_filter, FLICKER_FILTER);
-	ENHANCEMENT(flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE);
-	ENHANCEMENT(flicker_filter_2d, FLICKER_FILTER_2D);
-	ENHANCEMENT(tv_chroma_filter, TV_CHROMA_FILTER);
-	ENHANCEMENT(tv_luma_filter, TV_LUMA_FILTER);
+	ENHANCEMENT(&sdvo_state->tv, hpos, HPOS);
+	ENHANCEMENT(&sdvo_state->tv, vpos, VPOS);
+	ENHANCEMENT(&conn_state->tv, saturation, SATURATION);
+	ENHANCEMENT(&conn_state->tv, contrast, CONTRAST);
+	ENHANCEMENT(&conn_state->tv, hue, HUE);
+	ENHANCEMENT(&conn_state->tv, brightness, BRIGHTNESS);
+	ENHANCEMENT(&sdvo_state->tv, sharpness, SHARPNESS);
+	ENHANCEMENT(&sdvo_state->tv, flicker_filter, FLICKER_FILTER);
+	ENHANCEMENT(&sdvo_state->tv, flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE);
+	ENHANCEMENT(&sdvo_state->tv, flicker_filter_2d, FLICKER_FILTER_2D);
+	_ENHANCEMENT(sdvo_state->tv.chroma_filter, tv_chroma_filter, TV_CHROMA_FILTER);
+	_ENHANCEMENT(sdvo_state->tv.luma_filter, tv_luma_filter, TV_LUMA_FILTER);
 
 	if (enhancements.dot_crawl) {
 		if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_DOT_CRAWL, &response, 2))
 			return false;
 
-		intel_sdvo_connector->max_dot_crawl = 1;
-		intel_sdvo_connector->cur_dot_crawl = response & 0x1;
+		sdvo_state->tv.dot_crawl = response & 0x1;
 		intel_sdvo_connector->dot_crawl =
 			drm_property_create_range(dev, 0, "dot_crawl", 0, 1);
 		if (!intel_sdvo_connector->dot_crawl)
 			return false;
 
 		drm_object_attach_property(&connector->base,
-					      intel_sdvo_connector->dot_crawl,
-					      intel_sdvo_connector->cur_dot_crawl);
+					   intel_sdvo_connector->dot_crawl, 0);
 		DRM_DEBUG_KMS("dot crawl: current %d\n", response);
 	}
 
@@ -2861,11 +2884,12 @@ intel_sdvo_create_enhance_property_lvds(struct intel_sdvo *intel_sdvo,
 	struct drm_connector *connector = &intel_sdvo_connector->base.base;
 	uint16_t response, data_value[2];
 
-	ENHANCEMENT(brightness, BRIGHTNESS);
+	ENHANCEMENT(&connector->state->tv, brightness, BRIGHTNESS);
 
 	return true;
 }
 #undef ENHANCEMENT
+#undef _ENHANCEMENT
 
 static bool intel_sdvo_create_enhance_property(struct intel_sdvo *intel_sdvo,
 					       struct intel_sdvo_connector *intel_sdvo_connector)
-- 
2.7.4

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

Re: [PATCH v4 9/9] drm/i915: Convert intel_sdvo connector properties to atomic.

[Daniel Vetter]

On Wed, Apr 12, 2017 at 12:50:07PM +0200, Maarten Lankhorst wrote:
> SDVO was the last connector that's still using the legacy paths
> for properties, and this is with a reason!
> 
> This connector implements a lot of properties dynamically,
> and some of them shared with the digital connector state,
> so sdvo_connector_state subclasses intel_digital_connector_state.
> 
> set_property had a lot of validation, but this is handled in the
> drm core, so most of the validation can die off. The properties
> are written right before enabling the connector, since there is no
> good way to update the properties without crtc.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Yeah, somewhat silly that we can't just reuse the core decoding for all of
these, but oh well. For the series, except where I dropped some comments

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

For the others r-b once those comments are addressed, I don't expect a
huge impact on those (e.g. as long as we register the scaling property
from within panel_init you can keep the allowed_pfit_mode_mask stuff
you're adding here).
-Daniel


> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |  40 ---
>  drivers/gpu/drm/i915/intel_display.c |  37 ---
>  drivers/gpu/drm/i915/intel_drv.h     |   6 -
>  drivers/gpu/drm/i915/intel_sdvo.c    | 538 ++++++++++++++++++-----------------
>  4 files changed, 281 insertions(+), 340 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index eb72166675f0..f068b623cf10 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -36,46 +36,6 @@
>  #include "intel_drv.h"
>  
>  /**
> - * intel_connector_atomic_get_property - fetch legacy connector property value
> - * @connector: connector to fetch property for
> - * @state: state containing the property value
> - * @property: property to look up
> - * @val: pointer to write property value into
> - *
> - * The DRM core does not store shadow copies of properties for
> - * atomic-capable drivers.  This entrypoint is used to fetch
> - * the current value of a driver-specific connector property.
> - *
> - * This is a intermediary solution until all connectors are
> - * converted to support full atomic properties.
> - */
> -int intel_connector_atomic_get_property(struct drm_connector *connector,
> -					const struct drm_connector_state *state,
> -					struct drm_property *property,
> -					uint64_t *val)
> -{
> -	int i;
> -
> -	/*
> -	 * TODO: We only have atomic modeset for planes at the moment, so the
> -	 * crtc/connector code isn't quite ready yet.  Until it's ready,
> -	 * continue to look up all property values in the DRM's shadow copy
> -	 * in obj->properties->values[].
> -	 *
> -	 * When the crtc/connector state work matures, this function should
> -	 * be updated to read the values out of the state structure instead.
> -	 */
> -	for (i = 0; i < connector->base.properties->count; i++) {
> -		if (connector->base.properties->properties[i] == property) {
> -			*val = connector->base.properties->values[i];
> -			return 0;
> -		}
> -	}
> -
> -	return -EINVAL;
> -}
> -
> -/**
>   * intel_digital_connector_atomic_get_property - hook for connector->atomic_get_property.
>   * @connector: Connector to get the property for.
>   * @state: Connector state to retrieve the property from.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5eb4ddb04797..c2c367b90c0a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13075,43 +13075,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	return 0;
>  }
>  
> -void intel_crtc_restore_mode(struct drm_crtc *crtc)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_atomic_state *state;
> -	struct drm_crtc_state *crtc_state;
> -	int ret;
> -
> -	state = drm_atomic_state_alloc(dev);
> -	if (!state) {
> -		DRM_DEBUG_KMS("[CRTC:%d:%s] crtc restore failed, out of memory",
> -			      crtc->base.id, crtc->name);
> -		return;
> -	}
> -
> -	state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;
> -
> -retry:
> -	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> -	ret = PTR_ERR_OR_ZERO(crtc_state);
> -	if (!ret) {
> -		if (!crtc_state->active)
> -			goto out;
> -
> -		crtc_state->mode_changed = true;
> -		ret = drm_atomic_commit(state);
> -	}
> -
> -	if (ret == -EDEADLK) {
> -		drm_atomic_state_clear(state);
> -		drm_modeset_backoff(state->acquire_ctx);
> -		goto retry;
> -	}
> -
> -out:
> -	drm_atomic_state_put(state);
> -}
> -
>  static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.set_config = drm_atomic_helper_set_config,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d1bba4790eae..d2118413dd18 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1318,7 +1318,6 @@ unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info
>  bool intel_has_pending_fb_unpin(struct drm_i915_private *dev_priv);
>  void intel_mark_busy(struct drm_i915_private *dev_priv);
>  void intel_mark_idle(struct drm_i915_private *dev_priv);
> -void intel_crtc_restore_mode(struct drm_crtc *crtc);
>  int intel_display_suspend(struct drm_device *dev);
>  void intel_pps_unlock_regs_wa(struct drm_i915_private *dev_priv);
>  void intel_encoder_destroy(struct drm_encoder *encoder);
> @@ -1879,11 +1878,6 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
>  void intel_tv_init(struct drm_i915_private *dev_priv);
>  
>  /* intel_atomic.c */
> -int intel_connector_atomic_get_property(struct drm_connector *connector,
> -					const struct drm_connector_state *state,
> -					struct drm_property *property,
> -					uint64_t *val);
> -
>  int intel_digital_connector_atomic_get_property(struct drm_connector *connector,
>  						const struct drm_connector_state *state,
>  						struct drm_property *property,
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 9855e4d48542..0aac7ce2b218 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -100,18 +100,6 @@ struct intel_sdvo {
>  	uint16_t hotplug_active;
>  
>  	/**
> -	 * This is used to select the color range of RBG outputs in HDMI mode.
> -	 * It is only valid when using TMDS encoding and 8 bit per color mode.
> -	 */
> -	uint32_t color_range;
> -	bool color_range_auto;
> -
> -	/**
> -	 * HDMI user specified aspect ratio
> -	 */
> -	enum hdmi_picture_aspect aspect_ratio;
> -
> -	/**
>  	 * This is set if we're going to treat the device as TV-out.
>  	 *
>  	 * While we have these nice friendly flags for output types that ought
> @@ -122,9 +110,6 @@ struct intel_sdvo {
>  
>  	enum port port;
>  
> -	/* This is for current tv format name */
> -	int tv_format_index;
> -
>  	/**
>  	 * This is set if we treat the device as HDMI, instead of DVI.
>  	 */
> @@ -159,8 +144,6 @@ struct intel_sdvo_connector {
>  	/* Mark the type of connector */
>  	uint16_t output_flag;
>  
> -	enum hdmi_force_audio force_audio;
> -
>  	/* This contains all current supported TV format */
>  	u8 tv_format_supported[TV_FORMAT_NUM];
>  	int   format_supported_num;
> @@ -187,24 +170,19 @@ struct intel_sdvo_connector {
>  	/* add the property for the SDVO-TV/LVDS */
>  	struct drm_property *brightness;
>  
> -	/* Add variable to record current setting for the above property */
> -	u32	left_margin, right_margin, top_margin, bottom_margin;
> -
>  	/* this is to get the range of margin.*/
> -	u32	max_hscan,  max_vscan;
> -	u32	max_hpos, cur_hpos;
> -	u32	max_vpos, cur_vpos;
> -	u32	cur_brightness, max_brightness;
> -	u32	cur_contrast,	max_contrast;
> -	u32	cur_saturation, max_saturation;
> -	u32	cur_hue,	max_hue;
> -	u32	cur_sharpness,	max_sharpness;
> -	u32	cur_flicker_filter,		max_flicker_filter;
> -	u32	cur_flicker_filter_adaptive,	max_flicker_filter_adaptive;
> -	u32	cur_flicker_filter_2d,		max_flicker_filter_2d;
> -	u32	cur_tv_chroma_filter,	max_tv_chroma_filter;
> -	u32	cur_tv_luma_filter,	max_tv_luma_filter;
> -	u32	cur_dot_crawl,	max_dot_crawl;
> +	u32 max_hscan, max_vscan;
> +};
> +
> +struct intel_sdvo_connector_state {
> +	/* base.base: tv.saturation/contrast/hue/brightness */
> +	struct intel_digital_connector_state base;
> +
> +	struct {
> +		unsigned overscan_h, overscan_v, hpos, vpos, sharpness;
> +		unsigned flicker_filter, flicker_filter_2d, flicker_filter_adaptive;
> +		unsigned chroma_filter, luma_filter, dot_crawl;
> +	} tv;
>  };
>  
>  static struct intel_sdvo *to_sdvo(struct intel_encoder *encoder)
> @@ -217,9 +195,16 @@ static struct intel_sdvo *intel_attached_sdvo(struct drm_connector *connector)
>  	return to_sdvo(intel_attached_encoder(connector));
>  }
>  
> -static struct intel_sdvo_connector *to_intel_sdvo_connector(struct drm_connector *connector)
> +static struct intel_sdvo_connector *
> +to_intel_sdvo_connector(struct drm_connector *connector)
>  {
> -	return container_of(to_intel_connector(connector), struct intel_sdvo_connector, base);
> +	return container_of(connector, struct intel_sdvo_connector, base.base);
> +}
> +
> +static struct intel_sdvo_connector_state *
> +to_intel_sdvo_connector_state(struct drm_connector_state *conn_state)
> +{
> +	return container_of(conn_state, struct intel_sdvo_connector_state, base.base);
>  }
>  
>  static bool
> @@ -1035,12 +1020,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
>  					  sdvo_data, sizeof(sdvo_data));
>  }
>  
> -static bool intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo)
> +static bool intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo,
> +				     struct drm_connector_state *conn_state)
>  {
>  	struct intel_sdvo_tv_format format;
>  	uint32_t format_map;
>  
> -	format_map = 1 << intel_sdvo->tv_format_index;
> +	format_map = 1 << conn_state->tv.mode;
>  	memset(&format, 0, sizeof(format));
>  	memcpy(&format, &format_map, min(sizeof(format), sizeof(format_map)));
>  
> @@ -1127,8 +1113,8 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  				      struct drm_connector_state *conn_state)
>  {
>  	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
> -	struct intel_sdvo_connector *intel_sdvo_connector =
> -		to_intel_sdvo_connector(conn_state->connector);
> +	struct intel_sdvo_connector_state *intel_sdvo_state =
> +		to_intel_sdvo_connector_state(conn_state);
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	struct drm_display_mode *mode = &pipe_config->base.mode;
>  
> @@ -1167,14 +1153,14 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  	pipe_config->pixel_multiplier =
>  		intel_sdvo_get_pixel_multiplier(adjusted_mode);
>  
> -	if (intel_sdvo_connector->force_audio != HDMI_AUDIO_OFF_DVI)
> +	if (intel_sdvo_state->base.force_audio != HDMI_AUDIO_OFF_DVI)
>  		pipe_config->has_hdmi_sink = intel_sdvo->has_hdmi_monitor;
>  
> -	if (intel_sdvo_connector->force_audio == HDMI_AUDIO_ON ||
> -	    (intel_sdvo_connector->force_audio == HDMI_AUDIO_AUTO && intel_sdvo->has_hdmi_audio))
> +	if (intel_sdvo_state->base.force_audio == HDMI_AUDIO_ON ||
> +	    (intel_sdvo_state->base.force_audio == HDMI_AUDIO_AUTO && intel_sdvo->has_hdmi_audio))
>  		pipe_config->has_audio = true;
>  
> -	if (intel_sdvo->color_range_auto) {
> +	if (intel_sdvo_state->base.broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
>  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
>  		/* FIXME: This bit is only valid when using TMDS encoding and 8
>  		 * bit per color mode. */
> @@ -1183,7 +1169,7 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  			pipe_config->limited_color_range = true;
>  	} else {
>  		if (pipe_config->has_hdmi_sink &&
> -		    intel_sdvo->color_range == HDMI_COLOR_RANGE_16_235)
> +		    intel_sdvo_state->base.broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED)
>  			pipe_config->limited_color_range = true;
>  	}
>  
> @@ -1193,11 +1179,73 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
>  
>  	/* Set user selected PAR to incoming mode's member */
>  	if (intel_sdvo->is_hdmi)
> -		adjusted_mode->picture_aspect_ratio = intel_sdvo->aspect_ratio;
> +		adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio;
>  
>  	return true;
>  }
>  
> +#define UPDATE_PROPERTY(input, NAME) \
> +	do { \
> +		val = input; \
> +		intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_##NAME, &val, sizeof(val)); \
> +	} while (0)
> +
> +static void intel_sdvo_update_props(struct intel_sdvo *intel_sdvo,
> +				    struct intel_sdvo_connector_state *sdvo_state)
> +{
> +	struct drm_connector_state *conn_state = &sdvo_state->base.base;
> +	struct intel_sdvo_connector *intel_sdvo_conn =
> +		to_intel_sdvo_connector(conn_state->connector);
> +	uint16_t val;
> +
> +	if (intel_sdvo_conn->left)
> +		UPDATE_PROPERTY(sdvo_state->tv.overscan_h, OVERSCAN_H);
> +
> +	if (intel_sdvo_conn->top)
> +		UPDATE_PROPERTY(sdvo_state->tv.overscan_v, OVERSCAN_V);
> +
> +	if (intel_sdvo_conn->hpos)
> +		UPDATE_PROPERTY(sdvo_state->tv.hpos, HPOS);
> +
> +	if (intel_sdvo_conn->vpos)
> +		UPDATE_PROPERTY(sdvo_state->tv.vpos, VPOS);
> +
> +	if (intel_sdvo_conn->saturation)
> +		UPDATE_PROPERTY(conn_state->tv.saturation, SATURATION);
> +
> +	if (intel_sdvo_conn->contrast)
> +		UPDATE_PROPERTY(conn_state->tv.contrast, CONTRAST);
> +
> +	if (intel_sdvo_conn->hue)
> +		UPDATE_PROPERTY(conn_state->tv.hue, HUE);
> +
> +	if (intel_sdvo_conn->brightness)
> +		UPDATE_PROPERTY(conn_state->tv.brightness, BRIGHTNESS);
> +
> +	if (intel_sdvo_conn->sharpness)
> +		UPDATE_PROPERTY(sdvo_state->tv.sharpness, SHARPNESS);
> +
> +	if (intel_sdvo_conn->flicker_filter)
> +		UPDATE_PROPERTY(sdvo_state->tv.flicker_filter, FLICKER_FILTER);
> +
> +	if (intel_sdvo_conn->flicker_filter_2d)
> +		UPDATE_PROPERTY(sdvo_state->tv.flicker_filter_2d, FLICKER_FILTER_2D);
> +
> +	if (intel_sdvo_conn->flicker_filter_adaptive)
> +		UPDATE_PROPERTY(sdvo_state->tv.flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE);
> +
> +	if (intel_sdvo_conn->tv_chroma_filter)
> +		UPDATE_PROPERTY(sdvo_state->tv.chroma_filter, TV_CHROMA_FILTER);
> +
> +	if (intel_sdvo_conn->tv_luma_filter)
> +		UPDATE_PROPERTY(sdvo_state->tv.luma_filter, TV_LUMA_FILTER);
> +
> +	if (intel_sdvo_conn->dot_crawl)
> +		UPDATE_PROPERTY(sdvo_state->tv.dot_crawl, DOT_CRAWL);
> +
> +#undef UPDATE_PROPERTY
> +}
> +
>  static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
>  				  struct intel_crtc_state *crtc_state,
>  				  struct drm_connector_state *conn_state)
> @@ -1205,6 +1253,7 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
>  	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
> +	struct intel_sdvo_connector_state *sdvo_state = to_intel_sdvo_connector_state(conn_state);
>  	struct drm_display_mode *mode = &crtc_state->base.mode;
>  	struct intel_sdvo *intel_sdvo = to_sdvo(intel_encoder);
>  	u32 sdvox;
> @@ -1212,6 +1261,8 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
>  	struct intel_sdvo_dtd input_dtd, output_dtd;
>  	int rate;
>  
> +	intel_sdvo_update_props(intel_sdvo, sdvo_state);
> +
>  	/* First, set the input mapping for the first input to our controlled
>  	 * output. This is only correct if we're a single-input device, in
>  	 * which case the first input is the output from the appropriate SDVO
> @@ -1253,7 +1304,7 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
>  		intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
>  
>  	if (intel_sdvo->is_tv &&
> -	    !intel_sdvo_set_tv_format(intel_sdvo))
> +	    !intel_sdvo_set_tv_format(intel_sdvo, conn_state))
>  		return;
>  
>  	intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
> @@ -1885,6 +1936,7 @@ static const struct drm_display_mode sdvo_tv_modes[] = {
>  static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
>  {
>  	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
> +	const struct drm_connector_state *conn_state = connector->state;
>  	struct intel_sdvo_sdtv_resolution_request tv_res;
>  	uint32_t reply = 0, format_map = 0;
>  	int i;
> @@ -1895,7 +1947,7 @@ static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
>  	/* Read the list of supported input resolutions for the selected TV
>  	 * format.
>  	 */
> -	format_map = 1 << intel_sdvo->tv_format_index;
> +	format_map = 1 << conn_state->tv.mode;
>  	memcpy(&tv_res, &format_map,
>  	       min(sizeof(format_map), sizeof(struct intel_sdvo_sdtv_resolution_request)));
>  
> @@ -1985,187 +2037,120 @@ static void intel_sdvo_destroy(struct drm_connector *connector)
>  }
>  
>  static int
> -intel_sdvo_set_property(struct drm_connector *connector,
> -			struct drm_property *property,
> -			uint64_t val)
> +intel_sdvo_connector_atomic_get_property(struct drm_connector *connector,
> +					 const struct drm_connector_state *state,
> +					 struct drm_property *property,
> +					 uint64_t *val)
>  {
> -	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
>  	struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
> -	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> -	uint16_t temp_value;
> -	uint8_t cmd;
> -	int ret;
> -
> -	ret = drm_object_property_set_value(&connector->base, property, val);
> -	if (ret)
> -		return ret;
> -
> -	if (property == dev_priv->force_audio_property) {
> -		int i = val;
> -		bool has_audio, old_audio;
> -
> -		if (intel_sdvo_connector->force_audio == HDMI_AUDIO_AUTO)
> -			old_audio = intel_sdvo->has_hdmi_audio;
> -		else
> -			old_audio = intel_sdvo_connector->force_audio == HDMI_AUDIO_ON;
> -
> -		if (i == HDMI_AUDIO_AUTO)
> -			has_audio = intel_sdvo->has_hdmi_audio;
> -		else
> -			has_audio = (i == HDMI_AUDIO_ON);
> -
> -		intel_sdvo_connector->force_audio = i;
> -
> -		if (has_audio == old_audio)
> -			return 0;
> -
> -		goto done;
> -	}
> -
> -	if (property == dev_priv->broadcast_rgb_property) {
> -		bool old_auto = intel_sdvo->color_range_auto;
> -		uint32_t old_range = intel_sdvo->color_range;
> -
> -		switch (val) {
> -		case INTEL_BROADCAST_RGB_AUTO:
> -			intel_sdvo->color_range_auto = true;
> -			break;
> -		case INTEL_BROADCAST_RGB_FULL:
> -			intel_sdvo->color_range_auto = false;
> -			intel_sdvo->color_range = 0;
> -			break;
> -		case INTEL_BROADCAST_RGB_LIMITED:
> -			intel_sdvo->color_range_auto = false;
> -			/* FIXME: this bit is only valid when using TMDS
> -			 * encoding and 8 bit per color mode. */
> -			intel_sdvo->color_range = HDMI_COLOR_RANGE_16_235;
> -			break;
> -		default:
> -			return -EINVAL;
> -		}
> -
> -		if (old_auto == intel_sdvo->color_range_auto &&
> -		    old_range == intel_sdvo->color_range)
> -			return 0;
> -
> -		goto done;
> -	}
> -
> -	if (property == connector->dev->mode_config.aspect_ratio_property) {
> -		switch (val) {
> -		case DRM_MODE_PICTURE_ASPECT_NONE:
> -			intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> -			break;
> -		case DRM_MODE_PICTURE_ASPECT_4_3:
> -			intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
> -			break;
> -		case DRM_MODE_PICTURE_ASPECT_16_9:
> -			intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
> -			break;
> -		default:
> -			return -EINVAL;
> -		}
> -		goto done;
> -	}
> -
> -#define CHECK_PROPERTY(name, NAME) \
> -	if (intel_sdvo_connector->name == property) { \
> -		if (intel_sdvo_connector->cur_##name == temp_value) return 0; \
> -		if (intel_sdvo_connector->max_##name < temp_value) return -EINVAL; \
> -		cmd = SDVO_CMD_SET_##NAME; \
> -		intel_sdvo_connector->cur_##name = temp_value; \
> -		goto set_value; \
> -	}
> +	const struct intel_sdvo_connector_state *sdvo_state = to_intel_sdvo_connector_state((void *)state);
>  
>  	if (property == intel_sdvo_connector->tv_format) {
> -		if (val >= TV_FORMAT_NUM)
> -			return -EINVAL;
> -
> -		if (intel_sdvo->tv_format_index ==
> -		    intel_sdvo_connector->tv_format_supported[val])
> -			return 0;
> -
> -		intel_sdvo->tv_format_index = intel_sdvo_connector->tv_format_supported[val];
> -		goto done;
> -	} else if (IS_TV_OR_LVDS(intel_sdvo_connector)) {
> -		temp_value = val;
> -		if (intel_sdvo_connector->left == property) {
> -			drm_object_property_set_value(&connector->base,
> -							 intel_sdvo_connector->right, val);
> -			if (intel_sdvo_connector->left_margin == temp_value)
> -				return 0;
> +		int i;
>  
> -			intel_sdvo_connector->left_margin = temp_value;
> -			intel_sdvo_connector->right_margin = temp_value;
> -			temp_value = intel_sdvo_connector->max_hscan -
> -				intel_sdvo_connector->left_margin;
> -			cmd = SDVO_CMD_SET_OVERSCAN_H;
> -			goto set_value;
> -		} else if (intel_sdvo_connector->right == property) {
> -			drm_object_property_set_value(&connector->base,
> -							 intel_sdvo_connector->left, val);
> -			if (intel_sdvo_connector->right_margin == temp_value)
> -				return 0;
> +		for (i = 0; i < intel_sdvo_connector->format_supported_num; i++)
> +			if (state->tv.mode == intel_sdvo_connector->tv_format_supported[i]) {
> +				*val = i;
>  
> -			intel_sdvo_connector->left_margin = temp_value;
> -			intel_sdvo_connector->right_margin = temp_value;
> -			temp_value = intel_sdvo_connector->max_hscan -
> -				intel_sdvo_connector->left_margin;
> -			cmd = SDVO_CMD_SET_OVERSCAN_H;
> -			goto set_value;
> -		} else if (intel_sdvo_connector->top == property) {
> -			drm_object_property_set_value(&connector->base,
> -							 intel_sdvo_connector->bottom, val);
> -			if (intel_sdvo_connector->top_margin == temp_value)
>  				return 0;
> +			}
>  
> -			intel_sdvo_connector->top_margin = temp_value;
> -			intel_sdvo_connector->bottom_margin = temp_value;
> -			temp_value = intel_sdvo_connector->max_vscan -
> -				intel_sdvo_connector->top_margin;
> -			cmd = SDVO_CMD_SET_OVERSCAN_V;
> -			goto set_value;
> -		} else if (intel_sdvo_connector->bottom == property) {
> -			drm_object_property_set_value(&connector->base,
> -							 intel_sdvo_connector->top, val);
> -			if (intel_sdvo_connector->bottom_margin == temp_value)
> -				return 0;
> +		WARN_ON(1);
> +		*val = 0;
> +	} else if (property == intel_sdvo_connector->top ||
> +		   property == intel_sdvo_connector->bottom)
> +		*val = intel_sdvo_connector->max_vscan - sdvo_state->tv.overscan_v;
> +	else if (property == intel_sdvo_connector->left ||
> +		 property == intel_sdvo_connector->right)
> +		*val = intel_sdvo_connector->max_hscan - sdvo_state->tv.overscan_h;
> +	else if (property == intel_sdvo_connector->hpos)
> +		*val = sdvo_state->tv.hpos;
> +	else if (property == intel_sdvo_connector->vpos)
> +		*val = sdvo_state->tv.vpos;
> +	else if (property == intel_sdvo_connector->saturation)
> +		*val = state->tv.saturation;
> +	else if (property == intel_sdvo_connector->contrast)
> +		*val = state->tv.contrast;
> +	else if (property == intel_sdvo_connector->hue)
> +		*val = state->tv.hue;
> +	else if (property == intel_sdvo_connector->brightness)
> +		*val = state->tv.brightness;
> +	else if (property == intel_sdvo_connector->sharpness)
> +		*val = sdvo_state->tv.sharpness;
> +	else if (property == intel_sdvo_connector->flicker_filter)
> +		*val = sdvo_state->tv.flicker_filter;
> +	else if (property == intel_sdvo_connector->flicker_filter_2d)
> +		*val = sdvo_state->tv.flicker_filter_2d;
> +	else if (property == intel_sdvo_connector->flicker_filter_adaptive)
> +		*val = sdvo_state->tv.flicker_filter_adaptive;
> +	else if (property == intel_sdvo_connector->tv_chroma_filter)
> +		*val = sdvo_state->tv.chroma_filter;
> +	else if (property == intel_sdvo_connector->tv_luma_filter)
> +		*val = sdvo_state->tv.luma_filter;
> +	else if (property == intel_sdvo_connector->dot_crawl)
> +		*val = sdvo_state->tv.dot_crawl;
> +	else
> +		return intel_digital_connector_atomic_get_property(connector, state, property, val);
>  
> -			intel_sdvo_connector->top_margin = temp_value;
> -			intel_sdvo_connector->bottom_margin = temp_value;
> -			temp_value = intel_sdvo_connector->max_vscan -
> -				intel_sdvo_connector->top_margin;
> -			cmd = SDVO_CMD_SET_OVERSCAN_V;
> -			goto set_value;
> -		}
> -		CHECK_PROPERTY(hpos, HPOS)
> -		CHECK_PROPERTY(vpos, VPOS)
> -		CHECK_PROPERTY(saturation, SATURATION)
> -		CHECK_PROPERTY(contrast, CONTRAST)
> -		CHECK_PROPERTY(hue, HUE)
> -		CHECK_PROPERTY(brightness, BRIGHTNESS)
> -		CHECK_PROPERTY(sharpness, SHARPNESS)
> -		CHECK_PROPERTY(flicker_filter, FLICKER_FILTER)
> -		CHECK_PROPERTY(flicker_filter_2d, FLICKER_FILTER_2D)
> -		CHECK_PROPERTY(flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE)
> -		CHECK_PROPERTY(tv_chroma_filter, TV_CHROMA_FILTER)
> -		CHECK_PROPERTY(tv_luma_filter, TV_LUMA_FILTER)
> -		CHECK_PROPERTY(dot_crawl, DOT_CRAWL)
> -	}
> +	return 0;
> +}
>  
> -	return -EINVAL; /* unknown property */
> +static int
> +intel_sdvo_connector_atomic_set_property(struct drm_connector *connector,
> +					 struct drm_connector_state *state,
> +					 struct drm_property *property,
> +					 uint64_t val)
> +{
> +	struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
> +	struct intel_sdvo_connector_state *sdvo_state = to_intel_sdvo_connector_state(state);
>  
> -set_value:
> -	if (!intel_sdvo_set_value(intel_sdvo, cmd, &temp_value, 2))
> -		return -EIO;
> +	if (property == intel_sdvo_connector->tv_format) {
> +		state->tv.mode = intel_sdvo_connector->tv_format_supported[val];
>  
> +		if (state->crtc) {
> +			struct drm_crtc_state *crtc_state =
> +				drm_atomic_get_new_crtc_state(state->state, state->crtc);
>  
> -done:
> -	if (intel_sdvo->base.base.crtc)
> -		intel_crtc_restore_mode(intel_sdvo->base.base.crtc);
> +			crtc_state->connectors_changed = true;
> +		}
> +	} else if (property == intel_sdvo_connector->top ||
> +		   property == intel_sdvo_connector->bottom)
> +		/* Cannot set these independent from each other */
> +		sdvo_state->tv.overscan_v = intel_sdvo_connector->max_vscan - val;
> +	else if (property == intel_sdvo_connector->left ||
> +		 property == intel_sdvo_connector->right)
> +		/* Cannot set these independent from each other */
> +		sdvo_state->tv.overscan_h = intel_sdvo_connector->max_hscan - val;
> +	else if (property == intel_sdvo_connector->hpos)
> +		sdvo_state->tv.hpos = val;
> +	else if (property == intel_sdvo_connector->vpos)
> +		sdvo_state->tv.vpos = val;
> +	else if (property == intel_sdvo_connector->saturation)
> +		state->tv.saturation = val;
> +	else if (property == intel_sdvo_connector->contrast)
> +		state->tv.contrast = val;
> +	else if (property == intel_sdvo_connector->hue)
> +		state->tv.hue = val;
> +	else if (property == intel_sdvo_connector->brightness)
> +		state->tv.brightness = val;
> +	else if (property == intel_sdvo_connector->sharpness)
> +		sdvo_state->tv.sharpness = val;
> +	else if (property == intel_sdvo_connector->flicker_filter)
> +		sdvo_state->tv.flicker_filter = val;
> +	else if (property == intel_sdvo_connector->flicker_filter_2d)
> +		sdvo_state->tv.flicker_filter_2d = val;
> +	else if (property == intel_sdvo_connector->flicker_filter_adaptive)
> +		sdvo_state->tv.flicker_filter_adaptive = val;
> +	else if (property == intel_sdvo_connector->tv_chroma_filter)
> +		sdvo_state->tv.chroma_filter = val;
> +	else if (property == intel_sdvo_connector->tv_luma_filter)
> +		sdvo_state->tv.luma_filter = val;
> +	else if (property == intel_sdvo_connector->dot_crawl)
> +		sdvo_state->tv.dot_crawl = val;
> +	else
> +		return intel_digital_connector_atomic_set_property(connector, state, property, val);
>  
>  	return 0;
> -#undef CHECK_PROPERTY
>  }
>  
>  static int
> @@ -2193,22 +2178,61 @@ intel_sdvo_connector_unregister(struct drm_connector *connector)
>  	intel_connector_unregister(connector);
>  }
>  
> +static struct drm_connector_state *
> +intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
> +{
> +	struct intel_sdvo_connector_state *state;
> +
> +	state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return NULL;
> +
> +	__drm_atomic_helper_connector_duplicate_state(connector, &state->base.base);
> +	return &state->base.base;
> +}
> +
>  static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
>  	.dpms = drm_atomic_helper_connector_dpms,
>  	.detect = intel_sdvo_detect,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.set_property = intel_sdvo_set_property,
> -	.atomic_get_property = intel_connector_atomic_get_property,
> +	.set_property = drm_atomic_helper_connector_set_property,
> +	.atomic_get_property = intel_sdvo_connector_atomic_get_property,
> +	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
>  	.late_register = intel_sdvo_connector_register,
>  	.early_unregister = intel_sdvo_connector_unregister,
>  	.destroy = intel_sdvo_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_duplicate_state = intel_sdvo_connector_duplicate_state,
>  };
>  
> +static int intel_sdvo_atomic_check(struct drm_connector *conn,
> +				   struct drm_connector_state *new_conn_state)
> +{
> +	struct drm_atomic_state *state = new_conn_state->state;
> +	struct drm_connector_state *old_conn_state =
> +		drm_atomic_get_old_connector_state(state, conn);
> +	struct intel_sdvo_connector_state *old_state =
> +		to_intel_sdvo_connector_state(old_conn_state);
> +	struct intel_sdvo_connector_state *new_state =
> +		to_intel_sdvo_connector_state(new_conn_state);
> +
> +	if (new_conn_state->crtc &&
> +	    (memcmp(&old_state->tv, &new_state->tv, sizeof(old_state->tv)) ||
> +	     memcmp(&old_conn_state->tv, &new_conn_state->tv, sizeof(old_conn_state->tv)))) {
> +		struct drm_crtc_state *crtc_state =
> +			drm_atomic_get_new_crtc_state(new_conn_state->state,
> +						      new_conn_state->crtc);
> +
> +		crtc_state->connectors_changed = true;
> +	}
> +
> +	return intel_digital_connector_atomic_check(conn, new_conn_state);
> +}
> +
>  static const struct drm_connector_helper_funcs intel_sdvo_connector_helper_funcs = {
>  	.get_modes = intel_sdvo_get_modes,
>  	.mode_valid = intel_sdvo_mode_valid,
> +	.atomic_check = intel_sdvo_atomic_check,
>  };
>  
>  static void intel_sdvo_enc_destroy(struct drm_encoder *encoder)
> @@ -2400,25 +2424,28 @@ intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo,
>  	intel_attach_force_audio_property(&connector->base.base);
>  	if (INTEL_GEN(dev_priv) >= 4 && IS_MOBILE(dev_priv)) {
>  		intel_attach_broadcast_rgb_property(&connector->base.base);
> -		intel_sdvo->color_range_auto = true;
>  	}
>  	intel_attach_aspect_ratio_property(&connector->base.base);
> -	intel_sdvo->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
>  static struct intel_sdvo_connector *intel_sdvo_connector_alloc(void)
>  {
>  	struct intel_sdvo_connector *sdvo_connector;
> +	struct intel_sdvo_connector_state *conn_state;
>  
>  	sdvo_connector = kzalloc(sizeof(*sdvo_connector), GFP_KERNEL);
>  	if (!sdvo_connector)
>  		return NULL;
>  
> -	if (intel_connector_init(&sdvo_connector->base) < 0) {
> +	conn_state = kzalloc(sizeof(*conn_state), GFP_KERNEL);
> +	if (!conn_state) {
>  		kfree(sdvo_connector);
>  		return NULL;
>  	}
>  
> +	__drm_atomic_helper_connector_reset(&sdvo_connector->base.base,
> +					    &conn_state->base.base);
> +
>  	return sdvo_connector;
>  }
>  
> @@ -2710,31 +2737,30 @@ static bool intel_sdvo_tv_create_property(struct intel_sdvo *intel_sdvo,
>  				intel_sdvo_connector->tv_format, i,
>  				i, tv_format_names[intel_sdvo_connector->tv_format_supported[i]]);
>  
> -	intel_sdvo->tv_format_index = intel_sdvo_connector->tv_format_supported[0];
> -	drm_object_attach_property(&intel_sdvo_connector->base.base.base,
> -				      intel_sdvo_connector->tv_format, 0);
> +	intel_sdvo_connector->base.base.state->tv.mode = intel_sdvo_connector->tv_format_supported[0];
> +	drm_object_attach_property(&intel_sdvo_connector->base.base.base, 0, 0);
>  	return true;
>  
>  }
>  
> -#define ENHANCEMENT(name, NAME) do { \
> +#define _ENHANCEMENT(state_assignment, name, NAME) do { \
>  	if (enhancements.name) { \
>  		if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_MAX_##NAME, &data_value, 4) || \
>  		    !intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_##NAME, &response, 2)) \
>  			return false; \
> -		intel_sdvo_connector->max_##name = data_value[0]; \
> -		intel_sdvo_connector->cur_##name = response; \
>  		intel_sdvo_connector->name = \
>  			drm_property_create_range(dev, 0, #name, 0, data_value[0]); \
>  		if (!intel_sdvo_connector->name) return false; \
> +		state_assignment = response; \
>  		drm_object_attach_property(&connector->base, \
> -					      intel_sdvo_connector->name, \
> -					      intel_sdvo_connector->cur_##name); \
> +					   intel_sdvo_connector->name, 0); \
>  		DRM_DEBUG_KMS(#name ": max %d, default %d, current %d\n", \
>  			      data_value[0], data_value[1], response); \
>  	} \
>  } while (0)
>  
> +#define ENHANCEMENT(state, name, NAME) _ENHANCEMENT((state)->name, name, NAME)
> +
>  static bool
>  intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
>  				      struct intel_sdvo_connector *intel_sdvo_connector,
> @@ -2742,6 +2768,9 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
>  {
>  	struct drm_device *dev = intel_sdvo->base.base.dev;
>  	struct drm_connector *connector = &intel_sdvo_connector->base.base;
> +	struct drm_connector_state *conn_state = connector->state;
> +	struct intel_sdvo_connector_state *sdvo_state =
> +		to_intel_sdvo_connector_state(conn_state);
>  	uint16_t response, data_value[2];
>  
>  	/* when horizontal overscan is supported, Add the left/right  property */
> @@ -2756,17 +2785,16 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
>  					  &response, 2))
>  			return false;
>  
> +		sdvo_state->tv.overscan_h = response;
> +
>  		intel_sdvo_connector->max_hscan = data_value[0];
> -		intel_sdvo_connector->left_margin = data_value[0] - response;
> -		intel_sdvo_connector->right_margin = intel_sdvo_connector->left_margin;
>  		intel_sdvo_connector->left =
>  			drm_property_create_range(dev, 0, "left_margin", 0, data_value[0]);
>  		if (!intel_sdvo_connector->left)
>  			return false;
>  
>  		drm_object_attach_property(&connector->base,
> -					      intel_sdvo_connector->left,
> -					      intel_sdvo_connector->left_margin);
> +					   intel_sdvo_connector->left, 0);
>  
>  		intel_sdvo_connector->right =
>  			drm_property_create_range(dev, 0, "right_margin", 0, data_value[0]);
> @@ -2774,8 +2802,7 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
>  			return false;
>  
>  		drm_object_attach_property(&connector->base,
> -					      intel_sdvo_connector->right,
> -					      intel_sdvo_connector->right_margin);
> +					      intel_sdvo_connector->right, 0);
>  		DRM_DEBUG_KMS("h_overscan: max %d, "
>  			      "default %d, current %d\n",
>  			      data_value[0], data_value[1], response);
> @@ -2792,9 +2819,9 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
>  					  &response, 2))
>  			return false;
>  
> +		sdvo_state->tv.overscan_v = response;
> +
>  		intel_sdvo_connector->max_vscan = data_value[0];
> -		intel_sdvo_connector->top_margin = data_value[0] - response;
> -		intel_sdvo_connector->bottom_margin = intel_sdvo_connector->top_margin;
>  		intel_sdvo_connector->top =
>  			drm_property_create_range(dev, 0,
>  					    "top_margin", 0, data_value[0]);
> @@ -2802,8 +2829,7 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
>  			return false;
>  
>  		drm_object_attach_property(&connector->base,
> -					      intel_sdvo_connector->top,
> -					      intel_sdvo_connector->top_margin);
> +					   intel_sdvo_connector->top, 0);
>  
>  		intel_sdvo_connector->bottom =
>  			drm_property_create_range(dev, 0,
> @@ -2812,40 +2838,37 @@ intel_sdvo_create_enhance_property_tv(struct intel_sdvo *intel_sdvo,
>  			return false;
>  
>  		drm_object_attach_property(&connector->base,
> -					      intel_sdvo_connector->bottom,
> -					      intel_sdvo_connector->bottom_margin);
> +					      intel_sdvo_connector->bottom, 0);
>  		DRM_DEBUG_KMS("v_overscan: max %d, "
>  			      "default %d, current %d\n",
>  			      data_value[0], data_value[1], response);
>  	}
>  
> -	ENHANCEMENT(hpos, HPOS);
> -	ENHANCEMENT(vpos, VPOS);
> -	ENHANCEMENT(saturation, SATURATION);
> -	ENHANCEMENT(contrast, CONTRAST);
> -	ENHANCEMENT(hue, HUE);
> -	ENHANCEMENT(sharpness, SHARPNESS);
> -	ENHANCEMENT(brightness, BRIGHTNESS);
> -	ENHANCEMENT(flicker_filter, FLICKER_FILTER);
> -	ENHANCEMENT(flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE);
> -	ENHANCEMENT(flicker_filter_2d, FLICKER_FILTER_2D);
> -	ENHANCEMENT(tv_chroma_filter, TV_CHROMA_FILTER);
> -	ENHANCEMENT(tv_luma_filter, TV_LUMA_FILTER);
> +	ENHANCEMENT(&sdvo_state->tv, hpos, HPOS);
> +	ENHANCEMENT(&sdvo_state->tv, vpos, VPOS);
> +	ENHANCEMENT(&conn_state->tv, saturation, SATURATION);
> +	ENHANCEMENT(&conn_state->tv, contrast, CONTRAST);
> +	ENHANCEMENT(&conn_state->tv, hue, HUE);
> +	ENHANCEMENT(&conn_state->tv, brightness, BRIGHTNESS);
> +	ENHANCEMENT(&sdvo_state->tv, sharpness, SHARPNESS);
> +	ENHANCEMENT(&sdvo_state->tv, flicker_filter, FLICKER_FILTER);
> +	ENHANCEMENT(&sdvo_state->tv, flicker_filter_adaptive, FLICKER_FILTER_ADAPTIVE);
> +	ENHANCEMENT(&sdvo_state->tv, flicker_filter_2d, FLICKER_FILTER_2D);
> +	_ENHANCEMENT(sdvo_state->tv.chroma_filter, tv_chroma_filter, TV_CHROMA_FILTER);
> +	_ENHANCEMENT(sdvo_state->tv.luma_filter, tv_luma_filter, TV_LUMA_FILTER);
>  
>  	if (enhancements.dot_crawl) {
>  		if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_DOT_CRAWL, &response, 2))
>  			return false;
>  
> -		intel_sdvo_connector->max_dot_crawl = 1;
> -		intel_sdvo_connector->cur_dot_crawl = response & 0x1;
> +		sdvo_state->tv.dot_crawl = response & 0x1;
>  		intel_sdvo_connector->dot_crawl =
>  			drm_property_create_range(dev, 0, "dot_crawl", 0, 1);
>  		if (!intel_sdvo_connector->dot_crawl)
>  			return false;
>  
>  		drm_object_attach_property(&connector->base,
> -					      intel_sdvo_connector->dot_crawl,
> -					      intel_sdvo_connector->cur_dot_crawl);
> +					   intel_sdvo_connector->dot_crawl, 0);
>  		DRM_DEBUG_KMS("dot crawl: current %d\n", response);
>  	}
>  
> @@ -2861,11 +2884,12 @@ intel_sdvo_create_enhance_property_lvds(struct intel_sdvo *intel_sdvo,
>  	struct drm_connector *connector = &intel_sdvo_connector->base.base;
>  	uint16_t response, data_value[2];
>  
> -	ENHANCEMENT(brightness, BRIGHTNESS);
> +	ENHANCEMENT(&connector->state->tv, brightness, BRIGHTNESS);
>  
>  	return true;
>  }
>  #undef ENHANCEMENT
> +#undef _ENHANCEMENT
>  
>  static bool intel_sdvo_create_enhance_property(struct intel_sdvo *intel_sdvo,
>  					       struct intel_sdvo_connector *intel_sdvo_connector)
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Convert connector properties to atomic. (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Convert connector properties to atomic. (rev4)
URL   : https://patchwork.freedesktop.org/series/22634/
State : success

== Summary ==

Series 22634v4 drm/i915: Convert connector properties to atomic.
https://patchwork.freedesktop.org/api/1.0/series/22634/revisions/4/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:431s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:424s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:577s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:503s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:555s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:408s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:403s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:417s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:493s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:483s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:452s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:562s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:455s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:586s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:458s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:486s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:532s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:409s

02b830fd67a6a2681e7f64510c3256b51a85c21a drm-tip: 2017y-04m-12d-09h-19m-10s UTC integration manifest
73c9f58 drm/i915: Convert intel_sdvo connector properties to atomic.
40e0110 drm/i915: Handle force_audio correctly in intel_sdvo
458c6a8 drm/i915: Convert intel_hdmi connector properties to atomic
a72694f drm/i915: Convert intel_dp properties to atomic.
315e695 drm/i915: Make intel_dp->has_audio reflect hw state only
65737c6 drm/i915: Convert LVDS connector properties to atomic.
07de080 drm/i915: Convert DSI connector properties to atomic.
32e50f2 drm/i915: Add plumbing for digital connector state, v2.
19de4ba drm/atomic: Handle aspect ratio and scaling mode in core

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4489/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Convert connector properties to atomic. (rev5)

[Patchwork]

== Series Details ==

Series: drm/i915: Convert connector properties to atomic. (rev5)
URL   : https://patchwork.freedesktop.org/series/22634/
State : success

== Summary ==

Series 22634v5 drm/i915: Convert connector properties to atomic.
https://patchwork.freedesktop.org/api/1.0/series/22634/revisions/5/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#100652

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100652 https://bugs.freedesktop.org/show_bug.cgi?id=100652

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:432s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:430s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:568s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:511s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:548s
fi-byt-j1900     total:278  pass:253  dwarn:1   dfail:0   fail:0   skip:24  time:492s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:477s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:408s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:403s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:425s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:496s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:453s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:576s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:455s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:573s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:460s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:498s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:433s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:528s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:398s

6184edce6665aee9c9131149a7b9314a1313eaf9 drm-tip: 2017y-04m-13d-08h-27m-10s UTC integration manifest
7d21b5d drm/i915: Convert intel_sdvo connector properties to atomic.
67ccef1 drm/i915: Handle force_audio correctly in intel_sdvo
aed63c8 drm/i915: Convert intel_hdmi connector properties to atomic
0bbbde2 drm/i915: Convert intel_dp properties to atomic.
1408545 drm/i915: Make intel_dp->has_audio reflect hw state only
83e7070 drm/i915: Convert LVDS connector properties to atomic.
8dc0a98 drm/i915: Convert DSI connector properties to atomic.
07b3fdf drm/i915: Add plumbing for digital connector state, v2.
645cc7a drm/atomic: Handle aspect ratio and scaling mode in core, v2.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4500/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx