| Message ID | 20210604171723.10276-5-wse@tuxedocomputers.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add "activ bpc" drm property and use it in AMD and Intel driver | expand |
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 <wse@tuxedocomputers.com> > --- > 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
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 <wse@tuxedocomputers.com> >> --- >> 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 <wse@tuxedocomputers.com> >>> --- >>> 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 > amd-gfx@lists.freedesktop.org > 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 <wse@tuxedocomputers.com> >>>> --- >>>> 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 >> amd-gfx@lists.freedesktop.org >> 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 <wse@tuxedocomputers.com> > >>>> --- > >>>> 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().
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); + } } /*
This commits implements the "active bpc" drm property for the Intel GPU driver. Signed-off-by: Werner Sembach <wse@tuxedocomputers.com> --- 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(-)