This series moves the tidss to using new connectoe model, where the SoC driver (tidss) creates the connector and all the bridges are attached with the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR
Since the bridges do not create the connector, the bus_format and bus_flag is set via atomic hooks. Support format negotiations in the tfp410 and mhdp bridge drivers as a first step before moving the connector model.
Nikhil Devshatwar (7): drm/bridge: tfp410: Support format negotiation hooks drm/bridge: tfp410: Set input_bus_flags in atomic_check drm/bridge: mhdp8546: Add minimal format negotiation drm/bridge: mhdp8546: Set input_bus_flags from atomic_check drm/tidss: Set bus_format correctly from bridge/connector drm/tidss: Move to newer connector model drm/bridge: cdns-mhdp8546: Fix the interrupt enable/disable
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 75 +++++++++++++------ drivers/gpu/drm/bridge/ti-tfp410.c | 49 ++++++++++++ drivers/gpu/drm/tidss/tidss_drv.h | 3 + drivers/gpu/drm/tidss/tidss_encoder.c | 36 ++++----- drivers/gpu/drm/tidss/tidss_kms.c | 19 ++++- 5 files changed, 137 insertions(+), 45 deletions(-)
With new connector model, tfp410 will not create the connector and SoC driver will rely on format negotiation to setup the encoder format.
Support format negotiations hooks in the drm_bridge_funcs. Use helper functions for state management.
Output format is expected to be MEDIA_BUS_FMT_FIXED Input format is the one selected by the bridge from DT properties.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com ---
Notes: changes from v1: * Use only MEDIA_BUS_FMT_FIXED for output
drivers/gpu/drm/bridge/ti-tfp410.c | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index ba3fa2a9b8a4..3def9acba86b 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -204,12 +204,45 @@ static enum drm_mode_status tfp410_mode_valid(struct drm_bridge *bridge, return MODE_OK; }
+static u32 *tfp410_get_input_bus_fmts(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state, + u32 output_fmt, + unsigned int *num_input_fmts) +{ + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); + u32 *input_fmts; + + *num_input_fmts = 0; + + /* + * Output of tfp410 is DVI, which is TMDS. + * There is no media format defined for this. + * Using MEDIA_BUS_FMT_FIXED for now. + */ + if (output_fmt != MEDIA_BUS_FMT_FIXED) + return NULL; + + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL); + if (!input_fmts) + return NULL; + + *num_input_fmts = 1; + input_fmts[0] = dvi->bus_format; + return input_fmts; +} + static const struct drm_bridge_funcs tfp410_bridge_funcs = { .attach = tfp410_attach, .detach = tfp410_detach, .enable = tfp410_enable, .disable = tfp410_disable, .mode_valid = tfp410_mode_valid, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, + .atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts, };
static const struct drm_bridge_timings tfp410_default_timings = {
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Implement atomic_check hook for the same.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com --- drivers/gpu/drm/bridge/ti-tfp410.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index 3def9acba86b..4c536df003c8 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -233,6 +233,20 @@ static u32 *tfp410_get_input_bus_fmts(struct drm_bridge *bridge, return input_fmts; }
+int tfp410_atomic_check(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); + + /* + * There might be flags negotiation supported in future + * Set the bus flags in atomic_check statically for now + */ + bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags; +} + static const struct drm_bridge_funcs tfp410_bridge_funcs = { .attach = tfp410_attach, .detach = tfp410_detach, @@ -243,6 +257,7 @@ static const struct drm_bridge_funcs tfp410_bridge_funcs = { .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, .atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts, + .atomic_check = tfp410_atomic_check, };
static const struct drm_bridge_timings tfp410_default_timings = {
Hi Nikhil,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.10-rc6 next-20201201] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Nikhil-Devshatwar/drm-tidss-Use-new... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: mips-randconfig-r023-20201201 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project ac40a2d8f16b8a8c68fc811d67f647740e965cb8) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install mips cross compiling tool for clang build # apt-get install binutils-mips-linux-gnu # https://github.com/0day-ci/linux/commit/936e9f4e0dd9ad362b11e4cb6a240cdaec2b... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Nikhil-Devshatwar/drm-tidss-Use-new-connector-model-for-tidss/20201201-202125 git checkout 936e9f4e0dd9ad362b11e4cb6a240cdaec2b8b28 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/bridge/ti-tfp410.c:236:5: warning: no previous prototype for function 'tfp410_atomic_check' [-Wmissing-prototypes]
int tfp410_atomic_check(struct drm_bridge *bridge, ^ drivers/gpu/drm/bridge/ti-tfp410.c:236:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int tfp410_atomic_check(struct drm_bridge *bridge, ^ static drivers/gpu/drm/bridge/ti-tfp410.c:248:1: error: non-void function does not return a value [-Werror,-Wreturn-type] } ^ 1 warning and 1 error generated.
vim +/tfp410_atomic_check +236 drivers/gpu/drm/bridge/ti-tfp410.c
235
236 int tfp410_atomic_check(struct drm_bridge *bridge,
237 struct drm_bridge_state *bridge_state, 238 struct drm_crtc_state *crtc_state, 239 struct drm_connector_state *conn_state) 240 { 241 struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); 242 243 /* 244 * There might be flags negotiation supported in future 245 * Set the bus flags in atomic_check statically for now 246 */ 247 bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags; 248 } 249
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Implement atomic_check hook for the same.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com ---
Notes: changes from v4: * fix a warning Reported-by: kernel test robot lkp@intel.com
drivers/gpu/drm/bridge/ti-tfp410.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index 3def9acba86b..92963d12106b 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -233,6 +233,20 @@ static u32 *tfp410_get_input_bus_fmts(struct drm_bridge *bridge, return input_fmts; }
+static int tfp410_atomic_check(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); + + /* + * There might be flags negotiation supported in future + * Set the bus flags in atomic_check statically for now + */ + bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags; +} + static const struct drm_bridge_funcs tfp410_bridge_funcs = { .attach = tfp410_attach, .detach = tfp410_detach, @@ -243,6 +257,7 @@ static const struct drm_bridge_funcs tfp410_bridge_funcs = { .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, .atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts, + .atomic_check = tfp410_atomic_check, };
static const struct drm_bridge_timings tfp410_default_timings = {
On Thu, 3 Dec 2020 18:20:48 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Implement atomic_check hook for the same.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com
Notes: changes from v4: * fix a warning Reported-by: kernel test robot lkp@intel.com
drivers/gpu/drm/bridge/ti-tfp410.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index 3def9acba86b..92963d12106b 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -233,6 +233,20 @@ static u32 *tfp410_get_input_bus_fmts(struct drm_bridge *bridge, return input_fmts; }
+static int tfp410_atomic_check(struct drm_bridge *bridge,
struct drm_bridge_state *bridge_state,struct drm_crtc_state *crtc_state,struct drm_connector_state *conn_state)+{
- struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
- /*
* There might be flags negotiation supported in future* Set the bus flags in atomic_check statically for now*/- bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags;
The return statement is still missing :-).
+}
static const struct drm_bridge_funcs tfp410_bridge_funcs = { .attach = tfp410_attach, .detach = tfp410_detach, @@ -243,6 +257,7 @@ static const struct drm_bridge_funcs tfp410_bridge_funcs = { .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, .atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts,
- .atomic_check = tfp410_atomic_check,
};
static const struct drm_bridge_timings tfp410_default_timings = {
With new connector model, mhdp bridge will not create the connector and SoC driver will rely on format negotiation to setup the encoder format.
Support minimal format negotiations hooks in the drm_bridge_funcs. Complete format negotiation can be added based on EDID data. This patch adds the minimal required support to avoid failure after moving to new connector model.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com ---
Notes: changes from v1: * cosmetic fixes, commit message update
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..2cd809eed827 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2078,6 +2078,30 @@ cdns_mhdp_bridge_atomic_reset(struct drm_bridge *bridge) return &cdns_mhdp_state->base; }
+static u32 *cdns_mhdp_get_input_bus_fmts(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state, + u32 output_fmt, + unsigned int *num_input_fmts) +{ + u32 *input_fmts; + u32 default_bus_format = MEDIA_BUS_FMT_RGB121212_1X36; + + *num_input_fmts = 0; + + if (output_fmt != MEDIA_BUS_FMT_FIXED) + return NULL; + + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL); + if (!input_fmts) + return NULL; + + *num_input_fmts = 1; + input_fmts[0] = default_bus_format; + return input_fmts; +} + static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, struct drm_bridge_state *bridge_state, struct drm_crtc_state *crtc_state, @@ -2142,6 +2166,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = { .atomic_duplicate_state = cdns_mhdp_bridge_atomic_duplicate_state, .atomic_destroy_state = cdns_mhdp_bridge_atomic_destroy_state, .atomic_reset = cdns_mhdp_bridge_atomic_reset, + .atomic_get_input_bus_fmts = cdns_mhdp_get_input_bus_fmts, .detect = cdns_mhdp_bridge_detect, .get_edid = cdns_mhdp_bridge_get_edid, .hpd_enable = cdns_mhdp_bridge_hpd_enable,
On Tue, 1 Dec 2020 17:48:26 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
With new connector model, mhdp bridge will not create the connector and SoC driver will rely on format negotiation to setup the encoder format.
Support minimal format negotiations hooks in the drm_bridge_funcs. Complete format negotiation can be added based on EDID data. This patch adds the minimal required support to avoid failure after moving to new connector model.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com
Notes: changes from v1: * cosmetic fixes, commit message update
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..2cd809eed827 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2078,6 +2078,30 @@ cdns_mhdp_bridge_atomic_reset(struct drm_bridge *bridge) return &cdns_mhdp_state->base; }
+static u32 *cdns_mhdp_get_input_bus_fmts(struct drm_bridge *bridge,
struct drm_bridge_state *bridge_state,struct drm_crtc_state *crtc_state,struct drm_connector_state *conn_state,u32 output_fmt,unsigned int *num_input_fmts)+{
- u32 *input_fmts;
- u32 default_bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
- *num_input_fmts = 0;
- if (output_fmt != MEDIA_BUS_FMT_FIXED)
return NULL;- input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
- if (!input_fmts)
return NULL;- *num_input_fmts = 1;
- input_fmts[0] = default_bus_format;
Why not
input_fmts[0] = MEDIA_BUS_FMT_RGB121212_1X36;
?
This way you could get rid of the default_bus_format variable.
- return input_fmts;
+}
static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, struct drm_bridge_state *bridge_state, struct drm_crtc_state *crtc_state, @@ -2142,6 +2166,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = { .atomic_duplicate_state = cdns_mhdp_bridge_atomic_duplicate_state, .atomic_destroy_state = cdns_mhdp_bridge_atomic_destroy_state, .atomic_reset = cdns_mhdp_bridge_atomic_reset,
- .atomic_get_input_bus_fmts = cdns_mhdp_get_input_bus_fmts, .detect = cdns_mhdp_bridge_detect, .get_edid = cdns_mhdp_bridge_get_edid, .hpd_enable = cdns_mhdp_bridge_hpd_enable,
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com --- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 ++++++ drivers/gpu/drm/bridge/ti-tfp410.c | 1 + 2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 2cd809eed827..9c17e4bb517e 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2121,6 +2121,12 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, return -EINVAL; }
+ /* + * There might be flags negotiation supported in future + * Set the bus flags in atomic_check statically for now + */ + bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags; + mutex_unlock(&mhdp->link_mutex); return 0; } diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index 4c536df003c8..9035d2145a28 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -245,6 +245,7 @@ int tfp410_atomic_check(struct drm_bridge *bridge, * Set the bus flags in atomic_check statically for now */ bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags; + return 0; }
static const struct drm_bridge_funcs tfp410_bridge_funcs = {
On Tue, 1 Dec 2020 17:48:27 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 ++++++ drivers/gpu/drm/bridge/ti-tfp410.c | 1 + 2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 2cd809eed827..9c17e4bb517e 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2121,6 +2121,12 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, return -EINVAL; }
- /*
* There might be flags negotiation supported in future* Set the bus flags in atomic_check statically for now*/- bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags;
- mutex_unlock(&mhdp->link_mutex); return 0;
} diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index 4c536df003c8..9035d2145a28 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -245,6 +245,7 @@ int tfp410_atomic_check(struct drm_bridge *bridge, * Set the bus flags in atomic_check statically for now */ bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags;
- return 0;
And here is the return statement that was missing in patch 2 :-).
}
static const struct drm_bridge_funcs tfp410_bridge_funcs = {
On 11:32-20201204, Boris Brezillon wrote:
On Tue, 1 Dec 2020 17:48:27 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 ++++++ drivers/gpu/drm/bridge/ti-tfp410.c | 1 + 2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 2cd809eed827..9c17e4bb517e 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2121,6 +2121,12 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, return -EINVAL; }
- /*
* There might be flags negotiation supported in future* Set the bus flags in atomic_check statically for now*/- bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags;
- mutex_unlock(&mhdp->link_mutex); return 0;
} diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index 4c536df003c8..9035d2145a28 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -245,6 +245,7 @@ int tfp410_atomic_check(struct drm_bridge *bridge, * Set the bus flags in atomic_check statically for now */ bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags;
- return 0;
And here is the return statement that was missing in patch 2 :-).
Sorry. I guess I messed up while rebasing. Will fix this
Nikhil D
}
static const struct drm_bridge_funcs tfp410_bridge_funcs = {
On Tue, 1 Dec 2020 17:48:27 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 ++++++ drivers/gpu/drm/bridge/ti-tfp410.c | 1 + 2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 2cd809eed827..9c17e4bb517e 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2121,6 +2121,12 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, return -EINVAL; }
- /*
* There might be flags negotiation supported in future* Set the bus flags in atomic_check statically for now*/- bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags;
I'd go even further and replace the timings field in cdns_mhdp_platform_info by an input_bus_flags field, you'll then have the following assignment here.
if (mhdp->info) bridge_state->input_bus_cfg.flags = mhdp->info->input_bus_flags;
This way you no rely on the bridge->timings presence and can get rid of the mhdp->bridge.timings assignment in the probe path.
- mutex_unlock(&mhdp->link_mutex); return 0;
}
On 11:42-20201204, Boris Brezillon wrote:
On Tue, 1 Dec 2020 17:48:27 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
input_bus_flags are specified in drm_bridge_timings (legacy) as well as drm_bridge_state->input_bus_cfg.flags
The flags from the timings will be deprecated. Bridges are supposed to validate and set the bridge state flags from atomic_check.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 ++++++ drivers/gpu/drm/bridge/ti-tfp410.c | 1 + 2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 2cd809eed827..9c17e4bb517e 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2121,6 +2121,12 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, return -EINVAL; }
- /*
* There might be flags negotiation supported in future* Set the bus flags in atomic_check statically for now*/- bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags;
I'd go even further and replace the timings field in cdns_mhdp_platform_info by an input_bus_flags field, you'll then have the following assignment here.
if (mhdp->info) bridge_state->input_bus_cfg.flags = mhdp->info->input_bus_flags;
This way you no rely on the bridge->timings presence and can get rid of the mhdp->bridge.timings assignment in the probe path.
Okay, I'll update this patch
- mutex_unlock(&mhdp->link_mutex); return 0;
}
Remove the old code to iterate over the bridge chain, as this is already done by the framework. The bridge state should have the negotiated bus format and flags. Use these from the bridge's state. If the bridge does not support format negotiation, error out and fail.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com ---
Notes: changes from v3: * cosmetic updates changes from v2: * Remove the old code and use the flags from the bridge state
drivers/gpu/drm/tidss/tidss_encoder.c | 36 +++++++++++---------------- 1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c index e278a9c89476..5deb8102e4d3 100644 --- a/drivers/gpu/drm/tidss/tidss_encoder.c +++ b/drivers/gpu/drm/tidss/tidss_encoder.c @@ -21,37 +21,29 @@ static int tidss_encoder_atomic_check(struct drm_encoder *encoder, { struct drm_device *ddev = encoder->dev; struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state); - struct drm_display_info *di = &conn_state->connector->display_info; + struct drm_bridge_state *bstate; struct drm_bridge *bridge; - bool bus_flags_set = false;
dev_dbg(ddev->dev, "%s\n", __func__);
- /* - * Take the bus_flags from the first bridge that defines - * bridge timings, or from the connector's display_info if no - * bridge defines the timings. - */ - drm_for_each_bridge_in_chain(encoder, bridge) { - if (!bridge->timings) - continue; - - tcrtc_state->bus_flags = bridge->timings->input_bus_flags; - bus_flags_set = true; - break; + /* Copy the bus_format and flags from the first bridge's state */ + bridge = drm_bridge_chain_get_first_bridge(encoder); + bstate = drm_atomic_get_new_bridge_state(crtc_state->state, bridge); + if (!bstate) { + dev_err(ddev->dev, "Could not get the bridge state\n"); + return -EINVAL; }
- if (!di->bus_formats || di->num_bus_formats == 0) { - dev_err(ddev->dev, "%s: No bus_formats in connected display\n", - __func__); + tcrtc_state->bus_format = bstate->input_bus_cfg.format; + tcrtc_state->bus_flags = bstate->input_bus_cfg.flags; + + if (tcrtc_state->bus_format == 0 || + tcrtc_state->bus_format == MEDIA_BUS_FMT_FIXED) { + + dev_err(ddev->dev, "Bridge connected to the encoder did not specify media bus format\n"); return -EINVAL; }
- // XXX any cleaner way to set bus format and flags? - tcrtc_state->bus_format = di->bus_formats[0]; - if (!bus_flags_set) - tcrtc_state->bus_flags = di->bus_flags; - return 0; }
On Tue, 1 Dec 2020 17:48:28 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
Remove the old code to iterate over the bridge chain, as this is already done by the framework. The bridge state should have the negotiated bus format and flags. Use these from the bridge's state. If the bridge does not support format negotiation, error out and fail.
That'd be even better if you implement the bridge interface instead of the encoder one so we can get rid of the encoder_{helper}_funcs and use drm_simple_encoder_init().
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com
Notes: changes from v3: * cosmetic updates changes from v2: * Remove the old code and use the flags from the bridge state
drivers/gpu/drm/tidss/tidss_encoder.c | 36 +++++++++++---------------- 1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c index e278a9c89476..5deb8102e4d3 100644 --- a/drivers/gpu/drm/tidss/tidss_encoder.c +++ b/drivers/gpu/drm/tidss/tidss_encoder.c @@ -21,37 +21,29 @@ static int tidss_encoder_atomic_check(struct drm_encoder *encoder, { struct drm_device *ddev = encoder->dev; struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state);
- struct drm_display_info *di = &conn_state->connector->display_info;
- struct drm_bridge_state *bstate; struct drm_bridge *bridge;
bool bus_flags_set = false;
dev_dbg(ddev->dev, "%s\n", __func__);
/*
* Take the bus_flags from the first bridge that defines* bridge timings, or from the connector's display_info if no* bridge defines the timings.*/drm_for_each_bridge_in_chain(encoder, bridge) {
if (!bridge->timings)continue;tcrtc_state->bus_flags = bridge->timings->input_bus_flags;bus_flags_set = true;break;
- /* Copy the bus_format and flags from the first bridge's state */
- bridge = drm_bridge_chain_get_first_bridge(encoder);
- bstate = drm_atomic_get_new_bridge_state(crtc_state->state, bridge);
- if (!bstate) {
dev_err(ddev->dev, "Could not get the bridge state\n");return -EINVAL;
- if (!di->bus_formats || di->num_bus_formats == 0) {
dev_err(ddev->dev, "%s: No bus_formats in connected display\n",__func__);
- tcrtc_state->bus_format = bstate->input_bus_cfg.format;
- tcrtc_state->bus_flags = bstate->input_bus_cfg.flags;
- if (tcrtc_state->bus_format == 0 ||
tcrtc_state->bus_format == MEDIA_BUS_FMT_FIXED) {dev_err(ddev->dev, "Bridge connected to the encoder did not specify media bus format\n");
- // XXX any cleaner way to set bus format and flags?
- tcrtc_state->bus_format = di->bus_formats[0];
- if (!bus_flags_set)
tcrtc_state->bus_flags = di->bus_flags;- return 0;
}
Hi Boris,
On 04/12/2020 12:50, Boris Brezillon wrote:
On Tue, 1 Dec 2020 17:48:28 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
Remove the old code to iterate over the bridge chain, as this is already done by the framework. The bridge state should have the negotiated bus format and flags. Use these from the bridge's state. If the bridge does not support format negotiation, error out and fail.
That'd be even better if you implement the bridge interface instead of the encoder one so we can get rid of the encoder_{helper}_funcs and use drm_simple_encoder_init().
I'm a bit confused here. What should be the design here...
These formats need to be handled by the crtc (well, the display controller, which is modeled as the crtc). Should we have a tidss_crtc.c file, which implements a crtc, a simple encoder and a bridge? And just consider those three DRM components as different API parts of the display controller?
Tomi
On Fri, 4 Dec 2020 12:56:27 +0200 Tomi Valkeinen tomi.valkeinen@ti.com wrote:
Hi Boris,
On 04/12/2020 12:50, Boris Brezillon wrote:
On Tue, 1 Dec 2020 17:48:28 +0530 Nikhil Devshatwar nikhil.nd@ti.com wrote:
Remove the old code to iterate over the bridge chain, as this is already done by the framework. The bridge state should have the negotiated bus format and flags. Use these from the bridge's state. If the bridge does not support format negotiation, error out and fail.
That'd be even better if you implement the bridge interface instead of the encoder one so we can get rid of the encoder_{helper}_funcs and use drm_simple_encoder_init().
I'm a bit confused here. What should be the design here...
These formats need to be handled by the crtc (well, the display controller, which is modeled as the crtc). Should we have a tidss_crtc.c file, which implements a crtc, a simple encoder and a bridge? And just consider those three DRM components as different API parts of the display controller?
The idea is to hide the encoder abstraction from drivers as much as we can. We have to keep the encoder concept because that's what we expose to userspace, but drivers shouldn't have to worry about the distinction between the first bridge in the chain (what we currently call encoders) and other bridges. The bridge interface provides pretty much the same functionality, so all you need to do is turn your encoder driver into a bridge driver (see what we did for drivers/gpu/drm/imx/parallel-display.c), the only particularity here is that the bridge knows it's the first in the chain, and has access to the CRTC it's directly connected to.
IMHO, the less interfaces we keep the clearer it gets for our users. Getting rid of the encoder_{helper_}funcs in favor or bridge_ops would clarify the fact that any kind of encoder, no matter if it's the first in the chain or not, should be represented as a bridge object.
On 04/12/2020 13:12, Boris Brezillon wrote:
That'd be even better if you implement the bridge interface instead of the encoder one so we can get rid of the encoder_{helper}_funcs and use drm_simple_encoder_init().
I'm a bit confused here. What should be the design here...
These formats need to be handled by the crtc (well, the display controller, which is modeled as the crtc). Should we have a tidss_crtc.c file, which implements a crtc, a simple encoder and a bridge? And just consider those three DRM components as different API parts of the display controller?
The idea is to hide the encoder abstraction from drivers as much as we can. We have to keep the encoder concept because that's what we expose to userspace, but drivers shouldn't have to worry about the distinction between the first bridge in the chain (what we currently call encoders) and other bridges. The bridge interface provides pretty much the same functionality, so all you need to do is turn your encoder driver into a bridge driver (see what we did for drivers/gpu/drm/imx/parallel-display.c), the only particularity here is that the bridge knows it's the first in the chain, and has access to the CRTC it's directly connected to.
With a quick look, the imx parallel-display.c seems to be really part of the crtc. Shouldn't we then take one more step forward, and just combine the crtc, encoder and bridge (somehow)? That's kind of what parallel-display.c is doing, isn't it? It's directly poking the crtc state, but the code just happens to be in a separate file from the crtc.
Thinking about the TI HW, we have a bunch of internal IPs which are clearly bridges: HDMI, DSI, which get the pixel data from the display controller, and they have their own registers, so clearly independent bridges.
Then we have MIPI DPI, which doesn't really have its own registers, as it's just plain parallel RGB, the same as what is sent to e.g. HDMI and DSI bridges. However, there might be some muxes or regulators to set up to get the external signals working. So a bridge would be ok here too to handle that external side.
But in all the above cases, we have the display controller (crtc), which in all the cases needs to do e.g. pixel/bus format configuration. So why add direct crtc poking into the first bridges (HDMI, DSI, DPI), when we could just model the crtc as a bridge, and thus the first bridges wouldn't need to touch the crtc.
Tomi
On Fri, 4 Dec 2020 13:47:05 +0200 Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 04/12/2020 13:12, Boris Brezillon wrote:
That'd be even better if you implement the bridge interface instead of the encoder one so we can get rid of the encoder_{helper}_funcs and use drm_simple_encoder_init().
I'm a bit confused here. What should be the design here...
These formats need to be handled by the crtc (well, the display controller, which is modeled as the crtc). Should we have a tidss_crtc.c file, which implements a crtc, a simple encoder and a bridge? And just consider those three DRM components as different API parts of the display controller?
The idea is to hide the encoder abstraction from drivers as much as we can. We have to keep the encoder concept because that's what we expose to userspace, but drivers shouldn't have to worry about the distinction between the first bridge in the chain (what we currently call encoders) and other bridges. The bridge interface provides pretty much the same functionality, so all you need to do is turn your encoder driver into a bridge driver (see what we did for drivers/gpu/drm/imx/parallel-display.c), the only particularity here is that the bridge knows it's the first in the chain, and has access to the CRTC it's directly connected to.
With a quick look, the imx parallel-display.c seems to be really part of the crtc. Shouldn't we then take one more step forward, and just combine the crtc, encoder and bridge (somehow)? That's kind of what parallel-display.c is doing, isn't it? It's directly poking the crtc state, but the code just happens to be in a separate file from the crtc.
Right. If we want to keep the code split, the logic should probably be reversed, with the CRTC driver checking the first bridge state to setup its internal state. Those DPI encoders are always a bit special, since they tend to be directly embedded in the block responsible for timing control (what we call CRTCs), so you're right, maybe we should model that as a CRTC+bridge pair.
Thinking about the TI HW, we have a bunch of internal IPs which are clearly bridges: HDMI, DSI, which get the pixel data from the display controller, and they have their own registers, so clearly independent bridges.
Then we have MIPI DPI, which doesn't really have its own registers, as it's just plain parallel RGB, the same as what is sent to e.g. HDMI and DSI bridges.
I still consider that one a bridge, even if the translation is almost transparent from a HW PoV.
However, there might be some muxes or regulators to set up to get the external signals working. So a bridge would be ok here too to handle that external side.
Exactly.
But in all the above cases, we have the display controller (crtc), which in all the cases needs to do e.g. pixel/bus format configuration. So why add direct crtc poking into the first bridges (HDMI, DSI, DPI), when we could just model the crtc as a bridge, and thus the first bridges wouldn't need to touch the crtc.
Yes, that's an option. I mean, you can already modify your CRTC logic to implement the bridge and CRTC interface and have your driver-specific CRTC object embed both a drm_crtc and a drm_bridge.
On 04/12/2020 15:54, Boris Brezillon wrote:
On Fri, 4 Dec 2020 13:47:05 +0200 Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 04/12/2020 13:12, Boris Brezillon wrote:
That'd be even better if you implement the bridge interface instead of the encoder one so we can get rid of the encoder_{helper}_funcs and use drm_simple_encoder_init().
I'm a bit confused here. What should be the design here...
These formats need to be handled by the crtc (well, the display controller, which is modeled as the crtc). Should we have a tidss_crtc.c file, which implements a crtc, a simple encoder and a bridge? And just consider those three DRM components as different API parts of the display controller?
The idea is to hide the encoder abstraction from drivers as much as we can. We have to keep the encoder concept because that's what we expose to userspace, but drivers shouldn't have to worry about the distinction between the first bridge in the chain (what we currently call encoders) and other bridges. The bridge interface provides pretty much the same functionality, so all you need to do is turn your encoder driver into a bridge driver (see what we did for drivers/gpu/drm/imx/parallel-display.c), the only particularity here is that the bridge knows it's the first in the chain, and has access to the CRTC it's directly connected to.
With a quick look, the imx parallel-display.c seems to be really part of the crtc. Shouldn't we then take one more step forward, and just combine the crtc, encoder and bridge (somehow)? That's kind of what parallel-display.c is doing, isn't it? It's directly poking the crtc state, but the code just happens to be in a separate file from the crtc.
Right. If we want to keep the code split, the logic should probably be reversed, with the CRTC driver checking the first bridge state to setup its internal state. Those DPI encoders are always a bit special, since they tend to be directly embedded in the block responsible for timing control (what we call CRTCs), so you're right, maybe we should model that as a CRTC+bridge pair.
Thinking about the TI HW, we have a bunch of internal IPs which are clearly bridges: HDMI, DSI, which get the pixel data from the display controller, and they have their own registers, so clearly independent bridges.
Then we have MIPI DPI, which doesn't really have its own registers, as it's just plain parallel RGB, the same as what is sent to e.g. HDMI and DSI bridges.
I still consider that one a bridge, even if the translation is almost transparent from a HW PoV.
However, there might be some muxes or regulators to set up to get the external signals working. So a bridge would be ok here too to handle that external side.
Exactly.
But in all the above cases, we have the display controller (crtc), which in all the cases needs to do e.g. pixel/bus format configuration. So why add direct crtc poking into the first bridges (HDMI, DSI, DPI), when we could just model the crtc as a bridge, and thus the first bridges wouldn't need to touch the crtc.
Yes, that's an option. I mean, you can already modify your CRTC logic to implement the bridge and CRTC interface and have your driver-specific CRTC object embed both a drm_crtc and a drm_bridge.
Alright, thanks for the clarifications!
I think the best first step here is what you proposed, use the bridge interface and drm_simple_encoder_init(), as that should solve the issue at hand quite neatly.
Tomi
To be able to support connector operations across multiple bridges, it is recommended that the connector should be created by the SoC driver instead of the bridges.
Modify the tidss modesetting initialization sequence to create the connector and attach bridges with flag DRM_BRIDGE_ATTACH_NO_CONNECTOR
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com ---
Notes: changes from v1: * Add error handling
drivers/gpu/drm/tidss/tidss_drv.h | 3 +++ drivers/gpu/drm/tidss/tidss_kms.c | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h index 7de4bba52e6f..cfbf85a4d92b 100644 --- a/drivers/gpu/drm/tidss/tidss_drv.h +++ b/drivers/gpu/drm/tidss/tidss_drv.h @@ -27,6 +27,9 @@ struct tidss_device { unsigned int num_planes; struct drm_plane *planes[TIDSS_MAX_PLANES];
+ unsigned int num_connectors; + struct drm_connector *connectors[TIDSS_MAX_PORTS]; + spinlock_t wait_lock; /* protects the irq masks */ dispc_irq_t irq_mask; /* enabled irqs in addition to wait_list */ }; diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c index 09485c7f0d6f..1f5ae153b114 100644 --- a/drivers/gpu/drm/tidss/tidss_kms.c +++ b/drivers/gpu/drm/tidss/tidss_kms.c @@ -7,6 +7,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> +#include <drm/drm_bridge_connector.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_fb_helper.h> @@ -192,6 +193,7 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) for (i = 0; i < num_pipes; ++i) { struct tidss_plane *tplane; struct tidss_crtc *tcrtc; + struct drm_connector *connector; struct drm_encoder *enc; u32 hw_plane_id = feat->vid_order[tidss->num_planes]; int ret; @@ -222,11 +224,26 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss) return PTR_ERR(enc); }
- ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0); + ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); if (ret) { dev_err(tidss->dev, "bridge attach failed: %d\n", ret); return ret; } + + connector = drm_bridge_connector_init(&tidss->ddev, enc); + if (IS_ERR(connector)) { + dev_err(tidss->dev, "bridge_connector create failed\n"); + return PTR_ERR(connector); + } + + tidss->connectors[tidss->num_connectors++] = connector; + + ret = drm_connector_attach_encoder(connector, enc); + if (ret) { + dev_err(tidss->dev, "attaching encoder to connector failed\n"); + return ret; + } }
/* create overlay planes of the leftover planes */
When removing the tidss driver, there is a warning reported by kernel about an unhandled interrupt for mhdp driver.
[ 43.238895] irq 31: nobody cared (try booting with the "irqpoll" option) ... [snipped backtrace] [ 43.330735] handlers: [ 43.333020] [<000000005367c4f9>] irq_default_primary_handler threaded [<000000007e02b601>] cdns_mhdp_irq_handler [cdns_mhdp8546] [ 43.344607] Disabling IRQ #31
This happens because as part of cdns_mhdp_bridge_hpd_disable, driver tries to disable the interrupts. While disabling the SW_EVENT interrupts, it accidentally enables the MBOX interrupts, which are not handled by the driver.
Fix this with a read-modify-write to update only required bits. Use the enable / disable function as required in other places.
Signed-off-by: Nikhil Devshatwar nikhil.nd@ti.com Reviewed-by: Swapnil Jakhade sjakhade@cadence.com Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com ---
Notes: changes from v2: * Fix the interrupt enable logic at other places in code * Reorder functions so that they can be used earlier in the program
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 9c17e4bb517e..d0ed950f4f87 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -52,6 +52,26 @@
#include "cdns-mhdp8546-j721e.h"
+static void cdns_mhdp_bridge_hpd_enable(struct drm_bridge *bridge) +{ + struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge); + + /* Enable SW event interrupts */ + if (mhdp->bridge_attached) + writel(readl(mhdp->regs + CDNS_APB_INT_MASK) & + ~CDNS_APB_INT_MASK_SW_EVENT_INT, + mhdp->regs + CDNS_APB_INT_MASK); +} + +static void cdns_mhdp_bridge_hpd_disable(struct drm_bridge *bridge) +{ + struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge); + + writel(readl(mhdp->regs + CDNS_APB_INT_MASK) | + CDNS_APB_INT_MASK_SW_EVENT_INT, + mhdp->regs + CDNS_APB_INT_MASK); +} + static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp) { int ret, empty; @@ -747,9 +767,7 @@ static int cdns_mhdp_fw_activate(const struct firmware *fw, * MHDP_HW_STOPPED happens only due to driver removal when * bridge should already be detached. */ - if (mhdp->bridge_attached) - writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT, - mhdp->regs + CDNS_APB_INT_MASK); + cdns_mhdp_bridge_hpd_enable(&mhdp->bridge);
spin_unlock(&mhdp->start_lock);
@@ -1689,8 +1707,7 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
/* Enable SW event interrupts */ if (hw_ready) - writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT, - mhdp->regs + CDNS_APB_INT_MASK); + cdns_mhdp_bridge_hpd_enable(bridge);
return 0; } @@ -2146,23 +2163,6 @@ static struct edid *cdns_mhdp_bridge_get_edid(struct drm_bridge *bridge, return cdns_mhdp_get_edid(mhdp, connector); }
-static void cdns_mhdp_bridge_hpd_enable(struct drm_bridge *bridge) -{ - struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge); - - /* Enable SW event interrupts */ - if (mhdp->bridge_attached) - writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT, - mhdp->regs + CDNS_APB_INT_MASK); -} - -static void cdns_mhdp_bridge_hpd_disable(struct drm_bridge *bridge) -{ - struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge); - - writel(CDNS_APB_INT_MASK_SW_EVENT_INT, mhdp->regs + CDNS_APB_INT_MASK); -} - static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = { .atomic_enable = cdns_mhdp_atomic_enable, .atomic_disable = cdns_mhdp_atomic_disable,
dri-devel@lists.freedesktop.org
-
Boris Brezillon -
kernel test robot -
Nikhil Devshatwar -
Tomi Valkeinen