Re: [Intel-gfx] [PATCH] drm: Pass crtc to .best_encoder()
On 2018-06-26 11:01 AM, Harry Wentland wrote:
> On 2018-06-15 03:52 PM, Ville Syrjala wrote:
>> From: Ville Syrjälä
>>
>> To pick the correct MST encoder i915 wants to know which crtc is going
>> to be feeding us. To that end let's pass the crtc to the .best_encoder()
>> hook. The atomic variant already knows the crtc via the connector state,
>> but the non-atomic hooks is still being used by the fb_helper even on
>> atomic drivers.
>>
>> This allows us to fix up the possible_crtcs bitmask for the i915 MST
>> encoders. We have one encoder for each crtc+port combination, and thus
>> we have to know both the connector and the crtc to pick the right one.
>> This has only worked so far because every MST encoder lied in its
>> possible_crtcs bitmask that they can be driven by any crtc.
>>
>> I took the easy way out and passed NULL as the crtc for all the driver
>> internal uses of .best_encoder() in the amdgpu/radeon drivers. None of
>> the other drivers have such internal uses. The other callers
>> (crtc_helper, atomic_helper, fb_helper) will pass in the proper crtc.
>> but no one besides i915 will currently look at it.
>>
>> Cc: Alex Deucher
>> Cc: "Christian König"
>> Cc: "David (ChunMing) Zhou"
>> Cc: Harry Wentland
>> Cc: amd-...@lists.freedesktop.org
>> Signed-off-by: Ville Syrjälä
>
> amdgpu parts are
> Acked-by: Harry Wentland
>
Upgrading after proper review of the entire patch
Reviewed-by: Harry Wentland
Harry
> Harry
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 33 +
>> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 +-
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++--
>> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 5 +--
>> drivers/gpu/drm/ast/ast_mode.c | 3 +-
>> drivers/gpu/drm/bochs/bochs_kms.c | 3 +-
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 3 +-
>> drivers/gpu/drm/bridge/tc358767.c | 3 +-
>> drivers/gpu/drm/cirrus/cirrus_mode.c | 5 +--
>> drivers/gpu/drm/drm_atomic_helper.c| 16 ++---
>> drivers/gpu/drm/drm_crtc_helper.c | 3 +-
>> drivers/gpu/drm/drm_fb_helper.c| 41
>> --
>> drivers/gpu/drm/gma500/gma_display.c | 3 +-
>> drivers/gpu/drm/gma500/mdfld_dsi_output.c | 5 +--
>> drivers/gpu/drm/gma500/psb_intel_drv.h | 3 +-
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 3 +-
>> drivers/gpu/drm/i2c/tda998x_drv.c | 3 +-
>> drivers/gpu/drm/i915/intel_dp_mst.c| 9 +++--
>> drivers/gpu/drm/imx/imx-ldb.c | 5 +--
>> drivers/gpu/drm/imx/imx-tve.c | 5 +--
>> drivers/gpu/drm/imx/parallel-display.c | 5 +--
>> drivers/gpu/drm/mediatek/mtk_hdmi.c| 3 +-
>> drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +--
>> drivers/gpu/drm/msm/dsi/dsi_manager.c | 3 +-
>> drivers/gpu/drm/nouveau/dispnv50/disp.c| 3 +-
>> drivers/gpu/drm/nouveau/nouveau_connector.c| 3 +-
>> drivers/gpu/drm/qxl/qxl_display.c | 3 +-
>> drivers/gpu/drm/radeon/radeon_connectors.c | 40
>> +++--
>> drivers/gpu/drm/radeon/radeon_dp_mst.c | 5 +--
>> drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 3 +-
>> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 5 +--
>> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 5 +--
>> drivers/gpu/drm/udl/udl_connector.c| 3 +-
>> drivers/staging/vboxvideo/vbox_mode.c | 5 +--
>> include/drm/drm_atomic_helper.h| 3 +-
>> include/drm/drm_modeset_helper_vtables.h | 6 ++--
>> 36 files changed, 155 insertions(+), 106 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> index 8e66851eb427..3dfa50ec2589 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> @@ -135,7 +135,8 @@ int amdgpu_connector_get_monitor_bpc(struct
>> drm_connector *connector)
>> else {
>> const struct drm_connector_helper_funcs
>> *connector_funcs =
>> connector->helper_private;
>> -struct drm_encoder *encoder =
>> connector_funcs->best_encoder(connector);
>> +struct drm_encoder *encoder =
>> connector_funcs->best_encoder(connector,
>> +
>> NULL);
>> struct amdgpu_encoder *amdgpu_encoder =
>> to_amdgpu_encoder(encoder);
>> struct amdgpu_encoder_atom_dig *dig =
>> amdgpu_encoder->enc_priv;
>>
>> @@ -218,7 +219,7 @@ amdgpu_connector_update_scratch_regs(struct
>> drm_connector *con
Re: [Intel-gfx] [PATCH] drm: Pass crtc to .best_encoder()
On Tue, Jun 26, 2018 at 05:23:25PM +0200, Daniel Vetter wrote: > On Fri, Jun 15, 2018 at 10:52:21PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > To pick the correct MST encoder i915 wants to know which crtc is going > > to be feeding us. To that end let's pass the crtc to the .best_encoder() > > hook. The atomic variant already knows the crtc via the connector state, > > but the non-atomic hooks is still being used by the fb_helper even on > > atomic drivers. > > Can't we instead fix the fb helpers to use atomic_best_encoder? We'd need a connector state for that. Not sure we want to construct a fake one on demand or something. > Or just > drop that code, userspace doesn't really know any better either and just > needs to figure this out without the help of ->best_encoder, so it should > be possible. I have to admit I don't even remmber how userspace does this. OK, yeah so each connector has a list of encoders and each encoder has the possible_crtcs bitmask. So we should be able to just iterate through all the encoders for the connector. Not sure there couldn't be some some false positives if best_encoder() doesn't quite agree with the answer for some reason. But yeah, this does seem like a nicer option because we can then nuke the extra .best_encoder() hook for i915 mst. Now the question becomes whether I get to review the connecor->encoder_ids[] assignments in every driver. I suppose they should be mostly OK if userspace manages to work. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Pass crtc to .best_encoder()
On Fri, Jun 15, 2018 at 10:52:21PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> To pick the correct MST encoder i915 wants to know which crtc is going
> to be feeding us. To that end let's pass the crtc to the .best_encoder()
> hook. The atomic variant already knows the crtc via the connector state,
> but the non-atomic hooks is still being used by the fb_helper even on
> atomic drivers.
Can't we instead fix the fb helpers to use atomic_best_encoder? Or just
drop that code, userspace doesn't really know any better either and just
needs to figure this out without the help of ->best_encoder, so it should
be possible.
-Daniel
>
> This allows us to fix up the possible_crtcs bitmask for the i915 MST
> encoders. We have one encoder for each crtc+port combination, and thus
> we have to know both the connector and the crtc to pick the right one.
> This has only worked so far because every MST encoder lied in its
> possible_crtcs bitmask that they can be driven by any crtc.
>
> I took the easy way out and passed NULL as the crtc for all the driver
> internal uses of .best_encoder() in the amdgpu/radeon drivers. None of
> the other drivers have such internal uses. The other callers
> (crtc_helper, atomic_helper, fb_helper) will pass in the proper crtc.
> but no one besides i915 will currently look at it.
>
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "David (ChunMing) Zhou"
> Cc: Harry Wentland
> Cc: amd-...@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 33 +
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 +-
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++--
> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 5 +--
> drivers/gpu/drm/ast/ast_mode.c | 3 +-
> drivers/gpu/drm/bochs/bochs_kms.c | 3 +-
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 3 +-
> drivers/gpu/drm/bridge/tc358767.c | 3 +-
> drivers/gpu/drm/cirrus/cirrus_mode.c | 5 +--
> drivers/gpu/drm/drm_atomic_helper.c| 16 ++---
> drivers/gpu/drm/drm_crtc_helper.c | 3 +-
> drivers/gpu/drm/drm_fb_helper.c| 41
> --
> drivers/gpu/drm/gma500/gma_display.c | 3 +-
> drivers/gpu/drm/gma500/mdfld_dsi_output.c | 5 +--
> drivers/gpu/drm/gma500/psb_intel_drv.h | 3 +-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 3 +-
> drivers/gpu/drm/i2c/tda998x_drv.c | 3 +-
> drivers/gpu/drm/i915/intel_dp_mst.c| 9 +++--
> drivers/gpu/drm/imx/imx-ldb.c | 5 +--
> drivers/gpu/drm/imx/imx-tve.c | 5 +--
> drivers/gpu/drm/imx/parallel-display.c | 5 +--
> drivers/gpu/drm/mediatek/mtk_hdmi.c| 3 +-
> drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +--
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 3 +-
> drivers/gpu/drm/nouveau/dispnv50/disp.c| 3 +-
> drivers/gpu/drm/nouveau/nouveau_connector.c| 3 +-
> drivers/gpu/drm/qxl/qxl_display.c | 3 +-
> drivers/gpu/drm/radeon/radeon_connectors.c | 40 +++--
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 5 +--
> drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 3 +-
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 5 +--
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 5 +--
> drivers/gpu/drm/udl/udl_connector.c| 3 +-
> drivers/staging/vboxvideo/vbox_mode.c | 5 +--
> include/drm/drm_atomic_helper.h| 3 +-
> include/drm/drm_modeset_helper_vtables.h | 6 ++--
> 36 files changed, 155 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 8e66851eb427..3dfa50ec2589 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -135,7 +135,8 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
> *connector)
> else {
> const struct drm_connector_helper_funcs
> *connector_funcs =
> connector->helper_private;
> - struct drm_encoder *encoder =
> connector_funcs->best_encoder(connector);
> + struct drm_encoder *encoder =
> connector_funcs->best_encoder(connector,
> +
> NULL);
> struct amdgpu_encoder *amdgpu_encoder =
> to_amdgpu_encoder(encoder);
> struct amdgpu_encoder_atom_dig *dig =
> amdgpu_encoder->enc_priv;
>
> @@ -218,7 +219,7 @@ amdgpu_connector_update_scratch_regs(struct drm_connector
> *connector,
> bool c
Re: [Intel-gfx] [PATCH] drm: Pass crtc to .best_encoder()
On 2018-06-15 03:52 PM, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> To pick the correct MST encoder i915 wants to know which crtc is going
> to be feeding us. To that end let's pass the crtc to the .best_encoder()
> hook. The atomic variant already knows the crtc via the connector state,
> but the non-atomic hooks is still being used by the fb_helper even on
> atomic drivers.
>
> This allows us to fix up the possible_crtcs bitmask for the i915 MST
> encoders. We have one encoder for each crtc+port combination, and thus
> we have to know both the connector and the crtc to pick the right one.
> This has only worked so far because every MST encoder lied in its
> possible_crtcs bitmask that they can be driven by any crtc.
>
> I took the easy way out and passed NULL as the crtc for all the driver
> internal uses of .best_encoder() in the amdgpu/radeon drivers. None of
> the other drivers have such internal uses. The other callers
> (crtc_helper, atomic_helper, fb_helper) will pass in the proper crtc.
> but no one besides i915 will currently look at it.
>
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "David (ChunMing) Zhou"
> Cc: Harry Wentland
> Cc: amd-...@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä
amdgpu parts are
Acked-by: Harry Wentland
Harry
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 33 +
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 +-
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++--
> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 5 +--
> drivers/gpu/drm/ast/ast_mode.c | 3 +-
> drivers/gpu/drm/bochs/bochs_kms.c | 3 +-
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 3 +-
> drivers/gpu/drm/bridge/tc358767.c | 3 +-
> drivers/gpu/drm/cirrus/cirrus_mode.c | 5 +--
> drivers/gpu/drm/drm_atomic_helper.c| 16 ++---
> drivers/gpu/drm/drm_crtc_helper.c | 3 +-
> drivers/gpu/drm/drm_fb_helper.c| 41
> --
> drivers/gpu/drm/gma500/gma_display.c | 3 +-
> drivers/gpu/drm/gma500/mdfld_dsi_output.c | 5 +--
> drivers/gpu/drm/gma500/psb_intel_drv.h | 3 +-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 3 +-
> drivers/gpu/drm/i2c/tda998x_drv.c | 3 +-
> drivers/gpu/drm/i915/intel_dp_mst.c| 9 +++--
> drivers/gpu/drm/imx/imx-ldb.c | 5 +--
> drivers/gpu/drm/imx/imx-tve.c | 5 +--
> drivers/gpu/drm/imx/parallel-display.c | 5 +--
> drivers/gpu/drm/mediatek/mtk_hdmi.c| 3 +-
> drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +--
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 3 +-
> drivers/gpu/drm/nouveau/dispnv50/disp.c| 3 +-
> drivers/gpu/drm/nouveau/nouveau_connector.c| 3 +-
> drivers/gpu/drm/qxl/qxl_display.c | 3 +-
> drivers/gpu/drm/radeon/radeon_connectors.c | 40 +++--
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 5 +--
> drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 3 +-
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 5 +--
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 5 +--
> drivers/gpu/drm/udl/udl_connector.c| 3 +-
> drivers/staging/vboxvideo/vbox_mode.c | 5 +--
> include/drm/drm_atomic_helper.h| 3 +-
> include/drm/drm_modeset_helper_vtables.h | 6 ++--
> 36 files changed, 155 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 8e66851eb427..3dfa50ec2589 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -135,7 +135,8 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
> *connector)
> else {
> const struct drm_connector_helper_funcs
> *connector_funcs =
> connector->helper_private;
> - struct drm_encoder *encoder =
> connector_funcs->best_encoder(connector);
> + struct drm_encoder *encoder =
> connector_funcs->best_encoder(connector,
> +
> NULL);
> struct amdgpu_encoder *amdgpu_encoder =
> to_amdgpu_encoder(encoder);
> struct amdgpu_encoder_atom_dig *dig =
> amdgpu_encoder->enc_priv;
>
> @@ -218,7 +219,7 @@ amdgpu_connector_update_scratch_regs(struct drm_connector
> *connector,
> bool connected;
> int i;
>
> - best_encoder = connector_funcs->best_encoder(connector);
> + best_encoder = connector_funcs->best_encoder(connector, NULL);
>
> for (i = 0; i < DRM_CONNECTOR_M
[Intel-gfx] [PATCH] drm: Pass crtc to .best_encoder()
From: Ville Syrjälä
To pick the correct MST encoder i915 wants to know which crtc is going
to be feeding us. To that end let's pass the crtc to the .best_encoder()
hook. The atomic variant already knows the crtc via the connector state,
but the non-atomic hooks is still being used by the fb_helper even on
atomic drivers.
This allows us to fix up the possible_crtcs bitmask for the i915 MST
encoders. We have one encoder for each crtc+port combination, and thus
we have to know both the connector and the crtc to pick the right one.
This has only worked so far because every MST encoder lied in its
possible_crtcs bitmask that they can be driven by any crtc.
I took the easy way out and passed NULL as the crtc for all the driver
internal uses of .best_encoder() in the amdgpu/radeon drivers. None of
the other drivers have such internal uses. The other callers
(crtc_helper, atomic_helper, fb_helper) will pass in the proper crtc.
but no one besides i915 will currently look at it.
Cc: Alex Deucher
Cc: "Christian König"
Cc: "David (ChunMing) Zhou"
Cc: Harry Wentland
Cc: amd-...@lists.freedesktop.org
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 33 +
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 +-
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++--
.../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 5 +--
drivers/gpu/drm/ast/ast_mode.c | 3 +-
drivers/gpu/drm/bochs/bochs_kms.c | 3 +-
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 3 +-
drivers/gpu/drm/bridge/tc358767.c | 3 +-
drivers/gpu/drm/cirrus/cirrus_mode.c | 5 +--
drivers/gpu/drm/drm_atomic_helper.c| 16 ++---
drivers/gpu/drm/drm_crtc_helper.c | 3 +-
drivers/gpu/drm/drm_fb_helper.c| 41 --
drivers/gpu/drm/gma500/gma_display.c | 3 +-
drivers/gpu/drm/gma500/mdfld_dsi_output.c | 5 +--
drivers/gpu/drm/gma500/psb_intel_drv.h | 3 +-
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 3 +-
drivers/gpu/drm/i2c/tda998x_drv.c | 3 +-
drivers/gpu/drm/i915/intel_dp_mst.c| 9 +++--
drivers/gpu/drm/imx/imx-ldb.c | 5 +--
drivers/gpu/drm/imx/imx-tve.c | 5 +--
drivers/gpu/drm/imx/parallel-display.c | 5 +--
drivers/gpu/drm/mediatek/mtk_hdmi.c| 3 +-
drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +--
drivers/gpu/drm/msm/dsi/dsi_manager.c | 3 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c| 3 +-
drivers/gpu/drm/nouveau/nouveau_connector.c| 3 +-
drivers/gpu/drm/qxl/qxl_display.c | 3 +-
drivers/gpu/drm/radeon/radeon_connectors.c | 40 +++--
drivers/gpu/drm/radeon/radeon_dp_mst.c | 5 +--
drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 3 +-
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 5 +--
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 5 +--
drivers/gpu/drm/udl/udl_connector.c| 3 +-
drivers/staging/vboxvideo/vbox_mode.c | 5 +--
include/drm/drm_atomic_helper.h| 3 +-
include/drm/drm_modeset_helper_vtables.h | 6 ++--
36 files changed, 155 insertions(+), 106 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 8e66851eb427..3dfa50ec2589 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -135,7 +135,8 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
else {
const struct drm_connector_helper_funcs
*connector_funcs =
connector->helper_private;
- struct drm_encoder *encoder =
connector_funcs->best_encoder(connector);
+ struct drm_encoder *encoder =
connector_funcs->best_encoder(connector,
+
NULL);
struct amdgpu_encoder *amdgpu_encoder =
to_amdgpu_encoder(encoder);
struct amdgpu_encoder_atom_dig *dig =
amdgpu_encoder->enc_priv;
@@ -218,7 +219,7 @@ amdgpu_connector_update_scratch_regs(struct drm_connector
*connector,
bool connected;
int i;
- best_encoder = connector_funcs->best_encoder(connector);
+ best_encoder = connector_funcs->best_encoder(connector, NULL);
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
if (connector->encoder_ids[i] == 0)
@@ -358,7 +359,8 @@ static int amdgpu_connector_ddc_get_modes(struct
drm_connector *connector)
}
static struct drm_encoder *
-amdgpu_connector_best_single_encoder(struct drm_connector *conne
