This commits implements the "active bpc" drm property for the Intel GPU driver.
Signed-off-by: Werner Sembach <[email protected]>
---
drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++++++
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++--
drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++-
drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +++-
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 64e9107d70f7..f7898d9d7438 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10164,6 +10164,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc_state *new_crtc_state, *old_crtc_state;
struct intel_crtc *crtc;
+ struct drm_connector *connector;
+ struct drm_connector_state *new_conn_state;
u64 put_domains[I915_MAX_PIPES] = {};
intel_wakeref_t wakeref = 0;
int i;
@@ -10324,6 +10326,17 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
}
intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
+ /* Extract information from crtc to communicate it to userspace as connector properties */
+ for_each_new_connector_in_state(&state->base, connector, new_conn_state, i) {
+ struct drm_crtc *crtc = new_conn_state->crtc;
+ if (crtc) {
+ new_crtc_state = to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base, crtc));
+ new_conn_state->active_bpc = new_crtc_state->pipe_bpp / 3;
+ }
+ else
+ new_conn_state->active_bpc = 0;
+ }
+
/*
* Defer the cleanup of the old state to a separate worker to not
* impede the current task (userspace for blocking modesets) that
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 642c60f3d9b1..67826ba976ed 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4671,10 +4671,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
- if (HAS_GMCH(dev_priv))
+ if (HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 6, 10);
- else if (DISPLAY_VER(dev_priv) >= 5)
+ drm_connector_attach_active_bpc_property(connector, 6, 10);
+ }
+ else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
+ drm_connector_attach_active_bpc_property(connector, 6, 12);
+ }
/* Register HDMI colorspace for case of lspcon */
if (intel_bios_is_lspcon_present(dev_priv, port)) {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 2daa3f67791e..5a1869dc2210 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -844,8 +844,10 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
*/
connector->max_bpc_property =
intel_dp->attached_connector->base.max_bpc_property;
- if (connector->max_bpc_property)
+ if (connector->max_bpc_property) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
+ drm_connector_attach_active_bpc_property(connector, 6, 12);
+ }
return connector;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index d69f0a6dc26d..8af78b27b6ce 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2463,8 +2463,10 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
drm_object_attach_property(&connector->base,
connector->dev->mode_config.hdr_output_metadata_property, 0);
- if (!HAS_GMCH(dev_priv))
+ if (!HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 8, 12);
+ drm_connector_attach_active_bpc_property(connector, 8, 12);
+ }
}
/*
--
2.25.1
On Fri, Jun 04, 2021 at 07:17:23PM +0200, Werner Sembach wrote:
> This commits implements the "active bpc" drm property for the Intel GPU driver.
>
> Signed-off-by: Werner Sembach <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++++++
> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++--
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +++-
> 4 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 64e9107d70f7..f7898d9d7438 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10164,6 +10164,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> struct intel_crtc *crtc;
> + struct drm_connector *connector;
> + struct drm_connector_state *new_conn_state;
> u64 put_domains[I915_MAX_PIPES] = {};
> intel_wakeref_t wakeref = 0;
> int i;
> @@ -10324,6 +10326,17 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> }
> intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
>
> + /* Extract information from crtc to communicate it to userspace as connector properties */
> + for_each_new_connector_in_state(&state->base, connector, new_conn_state, i) {
> + struct drm_crtc *crtc = new_conn_state->crtc;
> + if (crtc) {
> + new_crtc_state = to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base, crtc));
intel_atomic_get_new_crtc_state()
> + new_conn_state->active_bpc = new_crtc_state->pipe_bpp / 3;
> + }
> + else
> + new_conn_state->active_bpc = 0;
> + }
This also seems too late. I think the whole thing should be
done somewhere around the normal swap_state() stuff.
> +
> /*
> * Defer the cleanup of the old state to a separate worker to not
> * impede the current task (userspace for blocking modesets) that
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 642c60f3d9b1..67826ba976ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4671,10 +4671,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> intel_attach_force_audio_property(connector);
>
> intel_attach_broadcast_rgb_property(connector);
> - if (HAS_GMCH(dev_priv))
> + if (HAS_GMCH(dev_priv)) {
> drm_connector_attach_max_bpc_property(connector, 6, 10);
> - else if (DISPLAY_VER(dev_priv) >= 5)
> + drm_connector_attach_active_bpc_property(connector, 6, 10);
> + }
> + else if (DISPLAY_VER(dev_priv) >= 5) {
> drm_connector_attach_max_bpc_property(connector, 6, 12);
> + drm_connector_attach_active_bpc_property(connector, 6, 12);
> + }
>
> /* Register HDMI colorspace for case of lspcon */
> if (intel_bios_is_lspcon_present(dev_priv, port)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 2daa3f67791e..5a1869dc2210 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -844,8 +844,10 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
> */
> connector->max_bpc_property =
> intel_dp->attached_connector->base.max_bpc_property;
> - if (connector->max_bpc_property)
> + if (connector->max_bpc_property) {
> drm_connector_attach_max_bpc_property(connector, 6, 12);
> + drm_connector_attach_active_bpc_property(connector, 6, 12);
> + }
>
> return connector;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index d69f0a6dc26d..8af78b27b6ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2463,8 +2463,10 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> drm_object_attach_property(&connector->base,
> connector->dev->mode_config.hdr_output_metadata_property, 0);
>
> - if (!HAS_GMCH(dev_priv))
> + if (!HAS_GMCH(dev_priv)) {
> drm_connector_attach_max_bpc_property(connector, 8, 12);
> + drm_connector_attach_active_bpc_property(connector, 8, 12);
> + }
> }
>
> /*
> --
> 2.25.1
--
Ville Syrj?l?
Intel
Am 04.06.21 um 19:30 schrieb Ville Syrjälä:
> On Fri, Jun 04, 2021 at 07:17:23PM +0200, Werner Sembach wrote:
>> This commits implements the "active bpc" drm property for the Intel GPU driver.
>>
>> Signed-off-by: Werner Sembach <[email protected]>
>> ---
>> drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++++++
>> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++--
>> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++-
>> drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +++-
>> 4 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 64e9107d70f7..f7898d9d7438 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -10164,6 +10164,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>> struct intel_crtc *crtc;
>> + struct drm_connector *connector;
>> + struct drm_connector_state *new_conn_state;
>> u64 put_domains[I915_MAX_PIPES] = {};
>> intel_wakeref_t wakeref = 0;
>> int i;
>> @@ -10324,6 +10326,17 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>> }
>> intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
>>
>> + /* Extract information from crtc to communicate it to userspace as connector properties */
>> + for_each_new_connector_in_state(&state->base, connector, new_conn_state, i) {
>> + struct drm_crtc *crtc = new_conn_state->crtc;
>> + if (crtc) {
>> + new_crtc_state = to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base, crtc));
> intel_atomic_get_new_crtc_state()
Thanks, will use that.
>
>> + new_conn_state->active_bpc = new_crtc_state->pipe_bpp / 3;
>> + }
>> + else
>> + new_conn_state->active_bpc = 0;
>> + }
> This also seems too late. I think the whole thing should be
> done somewhere around the normal swap_state() stuff.
Ok, will look into it.
>
>> +
>> /*
>> * Defer the cleanup of the old state to a separate worker to not
>> * impede the current task (userspace for blocking modesets) that
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 642c60f3d9b1..67826ba976ed 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -4671,10 +4671,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>> intel_attach_force_audio_property(connector);
>>
>> intel_attach_broadcast_rgb_property(connector);
>> - if (HAS_GMCH(dev_priv))
>> + if (HAS_GMCH(dev_priv)) {
>> drm_connector_attach_max_bpc_property(connector, 6, 10);
>> - else if (DISPLAY_VER(dev_priv) >= 5)
>> + drm_connector_attach_active_bpc_property(connector, 6, 10);
>> + }
>> + else if (DISPLAY_VER(dev_priv) >= 5) {
>> drm_connector_attach_max_bpc_property(connector, 6, 12);
>> + drm_connector_attach_active_bpc_property(connector, 6, 12);
>> + }
>>
>> /* Register HDMI colorspace for case of lspcon */
>> if (intel_bios_is_lspcon_present(dev_priv, port)) {
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index 2daa3f67791e..5a1869dc2210 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -844,8 +844,10 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>> */
>> connector->max_bpc_property =
>> intel_dp->attached_connector->base.max_bpc_property;
>> - if (connector->max_bpc_property)
>> + if (connector->max_bpc_property) {
>> drm_connector_attach_max_bpc_property(connector, 6, 12);
>> + drm_connector_attach_active_bpc_property(connector, 6, 12);
>> + }
>>
>> return connector;
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index d69f0a6dc26d..8af78b27b6ce 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -2463,8 +2463,10 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>> drm_object_attach_property(&connector->base,
>> connector->dev->mode_config.hdr_output_metadata_property, 0);
>>
>> - if (!HAS_GMCH(dev_priv))
>> + if (!HAS_GMCH(dev_priv)) {
>> drm_connector_attach_max_bpc_property(connector, 8, 12);
>> + drm_connector_attach_active_bpc_property(connector, 8, 12);
>> + }
>> }
>>
>> /*
>> --
>> 2.25.1
Am 07.06.21 um 08:47 schrieb Werner Sembach:
>
> Am 04.06.21 um 19:30 schrieb Ville Syrjälä:
>> On Fri, Jun 04, 2021 at 07:17:23PM +0200, Werner Sembach wrote:
>>> This commits implements the "active bpc" drm property for the Intel
>>> GPU driver.
>>>
>>> Signed-off-by: Werner Sembach <[email protected]>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++++++
>>> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++--
>>> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++-
>>> drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +++-
>>> 4 files changed, 25 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>> index 64e9107d70f7..f7898d9d7438 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -10164,6 +10164,8 @@ static void intel_atomic_commit_tail(struct
>>> intel_atomic_state *state)
>>> struct drm_i915_private *dev_priv = to_i915(dev);
>>> struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>>> struct intel_crtc *crtc;
>>> + struct drm_connector *connector;
>>> + struct drm_connector_state *new_conn_state;
>>> u64 put_domains[I915_MAX_PIPES] = {};
>>> intel_wakeref_t wakeref = 0;
>>> int i;
>>> @@ -10324,6 +10326,17 @@ static void intel_atomic_commit_tail(struct
>>> intel_atomic_state *state)
>>> }
>>> intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
>>> + /* Extract information from crtc to communicate it to
>>> userspace as connector properties */
>>> + for_each_new_connector_in_state(&state->base, connector,
>>> new_conn_state, i) {
>>> + struct drm_crtc *crtc = new_conn_state->crtc;
>>> + if (crtc) {
>>> + new_crtc_state =
>>> to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base, crtc));
>> intel_atomic_get_new_crtc_state()
> Thanks, will use that.
>>
>>> + new_conn_state->active_bpc = new_crtc_state->pipe_bpp / 3;
>>> + }
>>> + else
>>> + new_conn_state->active_bpc = 0;
>>> + }
>> This also seems too late. I think the whole thing should be
>> done somewhere around the normal swap_state() stuff.
> Ok, will look into it.
So I tried to put it in intel_atomic_commit() after
drm_atomic_helper_swap_state() and before
INIT_WORK(&state->base.commit_work, intel_atomic_commit_work) (which
creates a worker for intel_atomic_commit_tail), but somewhere in
between, the connector_state seems to change: The bpc written with the
for_each_new_connector_in_state() loop, gets discarded.
>>
>>> +
>>> /*
>>> * Defer the cleanup of the old state to a separate worker to not
>>> * impede the current task (userspace for blocking modesets) that
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index 642c60f3d9b1..67826ba976ed 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -4671,10 +4671,14 @@ intel_dp_add_properties(struct intel_dp
>>> *intel_dp, struct drm_connector *connect
>>> intel_attach_force_audio_property(connector);
>>> intel_attach_broadcast_rgb_property(connector);
>>> - if (HAS_GMCH(dev_priv))
>>> + if (HAS_GMCH(dev_priv)) {
>>> drm_connector_attach_max_bpc_property(connector, 6, 10);
>>> - else if (DISPLAY_VER(dev_priv) >= 5)
>>> + drm_connector_attach_active_bpc_property(connector, 6, 10);
>>> + }
>>> + else if (DISPLAY_VER(dev_priv) >= 5) {
>>> drm_connector_attach_max_bpc_property(connector, 6, 12);
>>> + drm_connector_attach_active_bpc_property(connector, 6, 12);
>>> + }
>>> /* Register HDMI colorspace for case of lspcon */
>>> if (intel_bios_is_lspcon_present(dev_priv, port)) {
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>> index 2daa3f67791e..5a1869dc2210 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>> @@ -844,8 +844,10 @@ static struct drm_connector
>>> *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>>> */
>>> connector->max_bpc_property =
>>> intel_dp->attached_connector->base.max_bpc_property;
>>> - if (connector->max_bpc_property)
>>> + if (connector->max_bpc_property) {
>>> drm_connector_attach_max_bpc_property(connector, 6, 12);
>>> + drm_connector_attach_active_bpc_property(connector, 6, 12);
>>> + }
>>> return connector;
>>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
>>> b/drivers/gpu/drm/i915/display/intel_hdmi.c
>>> index d69f0a6dc26d..8af78b27b6ce 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>>> @@ -2463,8 +2463,10 @@ intel_hdmi_add_properties(struct intel_hdmi
>>> *intel_hdmi, struct drm_connector *c
>>> drm_object_attach_property(&connector->base,
>>> connector->dev->mode_config.hdr_output_metadata_property, 0);
>>> - if (!HAS_GMCH(dev_priv))
>>> + if (!HAS_GMCH(dev_priv)) {
>>> drm_connector_attach_max_bpc_property(connector, 8, 12);
>>> + drm_connector_attach_active_bpc_property(connector, 8, 12);
>>> + }
>>> }
>>> /*
>>> --
>>> 2.25.1
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 07.06.21 um 22:33 schrieb Werner Sembach:
> Am 07.06.21 um 08:47 schrieb Werner Sembach:
>>
>> Am 04.06.21 um 19:30 schrieb Ville Syrjälä:
>>> On Fri, Jun 04, 2021 at 07:17:23PM +0200, Werner Sembach wrote:
>>>> This commits implements the "active bpc" drm property for the Intel
>>>> GPU driver.
>>>>
>>>> Signed-off-by: Werner Sembach <[email protected]>
>>>> ---
>>>> drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++++++
>>>> drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++--
>>>> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 +++-
>>>> drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +++-
>>>> 4 files changed, 25 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>>> index 64e9107d70f7..f7898d9d7438 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>> @@ -10164,6 +10164,8 @@ static void intel_atomic_commit_tail(struct
>>>> intel_atomic_state *state)
>>>> struct drm_i915_private *dev_priv = to_i915(dev);
>>>> struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>>>> struct intel_crtc *crtc;
>>>> + struct drm_connector *connector;
>>>> + struct drm_connector_state *new_conn_state;
>>>> u64 put_domains[I915_MAX_PIPES] = {};
>>>> intel_wakeref_t wakeref = 0;
>>>> int i;
>>>> @@ -10324,6 +10326,17 @@ static void
>>>> intel_atomic_commit_tail(struct intel_atomic_state *state)
>>>> }
>>>> intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
>>>> + /* Extract information from crtc to communicate it to
>>>> userspace as connector properties */
>>>> + for_each_new_connector_in_state(&state->base, connector,
>>>> new_conn_state, i) {
>>>> + struct drm_crtc *crtc = new_conn_state->crtc;
>>>> + if (crtc) {
>>>> + new_crtc_state =
>>>> to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base,
>>>> crtc));
>>> intel_atomic_get_new_crtc_state()
>> Thanks, will use that.
>>>
>>>> + new_conn_state->active_bpc = new_crtc_state->pipe_bpp / 3;
>>>> + }
>>>> + else
>>>> + new_conn_state->active_bpc = 0;
>>>> + }
>>> This also seems too late. I think the whole thing should be
>>> done somewhere around the normal swap_state() stuff.
>> Ok, will look into it.
> So I tried to put it in intel_atomic_commit() after
> drm_atomic_helper_swap_state() and before
> INIT_WORK(&state->base.commit_work, intel_atomic_commit_work) (which
> creates a worker for intel_atomic_commit_tail), but somewhere in
> between, the connector_state seems to change: The bpc written with the
> for_each_new_connector_in_state() loop, gets discarded.
This was not the problem. Setting the drm property immutable made it
(also?) immutable from the driver context, which I didn't test separately.
Removed the immutable again and moved the loop.
>>>
>>>> +
>>>> /*
>>>> * Defer the cleanup of the old state to a separate worker to
>>>> not
>>>> * impede the current task (userspace for blocking modesets)
>>>> that
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> index 642c60f3d9b1..67826ba976ed 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> @@ -4671,10 +4671,14 @@ intel_dp_add_properties(struct intel_dp
>>>> *intel_dp, struct drm_connector *connect
>>>> intel_attach_force_audio_property(connector);
>>>> intel_attach_broadcast_rgb_property(connector);
>>>> - if (HAS_GMCH(dev_priv))
>>>> + if (HAS_GMCH(dev_priv)) {
>>>> drm_connector_attach_max_bpc_property(connector, 6, 10);
>>>> - else if (DISPLAY_VER(dev_priv) >= 5)
>>>> + drm_connector_attach_active_bpc_property(connector, 6, 10);
>>>> + }
>>>> + else if (DISPLAY_VER(dev_priv) >= 5) {
>>>> drm_connector_attach_max_bpc_property(connector, 6, 12);
>>>> + drm_connector_attach_active_bpc_property(connector, 6, 12);
>>>> + }
>>>> /* Register HDMI colorspace for case of lspcon */
>>>> if (intel_bios_is_lspcon_present(dev_priv, port)) {
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>>> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>>> index 2daa3f67791e..5a1869dc2210 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>>>> @@ -844,8 +844,10 @@ static struct drm_connector
>>>> *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>>>> */
>>>> connector->max_bpc_property =
>>>> intel_dp->attached_connector->base.max_bpc_property;
>>>> - if (connector->max_bpc_property)
>>>> + if (connector->max_bpc_property) {
>>>> drm_connector_attach_max_bpc_property(connector, 6, 12);
>>>> + drm_connector_attach_active_bpc_property(connector, 6, 12);
>>>> + }
>>>> return connector;
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> b/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> index d69f0a6dc26d..8af78b27b6ce 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> @@ -2463,8 +2463,10 @@ intel_hdmi_add_properties(struct intel_hdmi
>>>> *intel_hdmi, struct drm_connector *c
>>>> drm_object_attach_property(&connector->base,
>>>> connector->dev->mode_config.hdr_output_metadata_property, 0);
>>>> - if (!HAS_GMCH(dev_priv))
>>>> + if (!HAS_GMCH(dev_priv)) {
>>>> drm_connector_attach_max_bpc_property(connector, 8, 12);
>>>> + drm_connector_attach_active_bpc_property(connector, 8, 12);
>>>> + }
>>>> }
>>>> /*
>>>> --
>>>> 2.25.1
>> _______________________________________________
>> amd-gfx mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Tue, Jun 08, 2021 at 07:19:31PM +0200, Werner Sembach wrote:
>
> Am 07.06.21 um 22:33 schrieb Werner Sembach:
> > Am 07.06.21 um 08:47 schrieb Werner Sembach:
> >>
> >> Am 04.06.21 um 19:30 schrieb Ville Syrj?l?:
> >>> On Fri, Jun 04, 2021 at 07:17:23PM +0200, Werner Sembach wrote:
> >>>> This commits implements the "active bpc" drm property for the Intel
> >>>> GPU driver.
> >>>>
> >>>> Signed-off-by: Werner Sembach <[email protected]>
> >>>> ---
> >>>> ? drivers/gpu/drm/i915/display/intel_display.c | 13 +++++++++++++
> >>>> ? drivers/gpu/drm/i915/display/intel_dp.c????? |? 8 ++++++--
> >>>> ? drivers/gpu/drm/i915/display/intel_dp_mst.c? |? 4 +++-
> >>>> ? drivers/gpu/drm/i915/display/intel_hdmi.c??? |? 4 +++-
> >>>> ? 4 files changed, 25 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> >>>> b/drivers/gpu/drm/i915/display/intel_display.c
> >>>> index 64e9107d70f7..f7898d9d7438 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >>>> @@ -10164,6 +10164,8 @@ static void intel_atomic_commit_tail(struct
> >>>> intel_atomic_state *state)
> >>>> ????? struct drm_i915_private *dev_priv = to_i915(dev);
> >>>> ????? struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> >>>> ????? struct intel_crtc *crtc;
> >>>> +??? struct drm_connector *connector;
> >>>> +??? struct drm_connector_state *new_conn_state;
> >>>> ????? u64 put_domains[I915_MAX_PIPES] = {};
> >>>> ????? intel_wakeref_t wakeref = 0;
> >>>> ????? int i;
> >>>> @@ -10324,6 +10326,17 @@ static void
> >>>> intel_atomic_commit_tail(struct intel_atomic_state *state)
> >>>> ????? }
> >>>> ????? intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
> >>>> ? +??? /* Extract information from crtc to communicate it to
> >>>> userspace as connector properties */
> >>>> +??? for_each_new_connector_in_state(&state->base, connector,
> >>>> new_conn_state, i) {
> >>>> +??????? struct drm_crtc *crtc = new_conn_state->crtc;
> >>>> +??????? if (crtc) {
> >>>> +??????????? new_crtc_state =
> >>>> to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base,
> >>>> crtc));
> >>> intel_atomic_get_new_crtc_state()
> >> Thanks, will use that.
> >>>
> >>>> + new_conn_state->active_bpc = new_crtc_state->pipe_bpp / 3;
> >>>> +??????? }
> >>>> +??????? else
> >>>> +??????????? new_conn_state->active_bpc = 0;
> >>>> +??? }
> >>> This also seems too late. I think the whole thing should be
> >>> done somewhere around the normal swap_state() stuff.
> >> Ok, will look into it.
> > So I tried to put it in intel_atomic_commit() after
> > drm_atomic_helper_swap_state() and before
> > INIT_WORK(&state->base.commit_work, intel_atomic_commit_work) (which
> > creates a worker for intel_atomic_commit_tail), but somewhere in
> > between, the connector_state seems to change: The bpc written with the
> > for_each_new_connector_in_state() loop, gets discarded.
>
> This was not the problem. Setting the drm property immutable made it
> (also?) immutable from the driver context, which I didn't test separately.
>
> Removed the immutable again and moved the loop.
Immutable props are special. See __drm_object_property_get_value().
--
Ville Syrj?l?
Intel