Hello Rob,
We removed the bridge object for DisplayPort due to review comments in
patch set 1.
Added more details below.
-------- Original Message --------
Subject: Re: [DPU PATCH v3 3/5] drm/msm/dp: add displayPort driver
support
Date: 2019-12-02 08:48
From: Rob Clark <robdclark@xxxxxxxxx>
To: Chandan Uddaraju <chandanu@xxxxxxxxxxxxxx>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@xxxxxxxxxxxxxxx>, linux-arm-msm
<linux-arm-msm@xxxxxxxxxxxxxxx>, Abhinav Kumar
<abhinavk@xxxxxxxxxxxxxx>, Sean Paul <seanpaul@xxxxxxxxxxxx>,
dri-devel <dri-devel@xxxxxxxxxxxxxxxxxxxxx>, "Kristian H. Kristensen"
<hoegsberg@xxxxxxxxxx>, freedreno <freedreno@xxxxxxxxxxxxxxxxxxxxx>
On Mon, Dec 2, 2019 at 5:48 AM Chandan Uddaraju
<chandanu@xxxxxxxxxxxxxx> wrote:
Add the needed displayPort files to enable DP driver
on msm target.
"dp_display" module is the main module that calls into
other sub-modules. "dp_drm" file represents the interface
between DRM framework and DP driver.
changes in v2:
-- Update copyright markings on all relevant files.
-- Change pr_err() to DRM_ERROR()
-- Use APIs directly instead of function pointers.
-- Use drm_display_mode structure to store link parameters in the
driver.
-- Use macros for register definitions instead of hardcoded values.
-- Replace writel_relaxed/readl_relaxed with writel/readl
and remove memory barriers.
-- Remove unnecessary NULL checks.
-- Use drm helper functions for dpcd read/write.
-- Use DRM_DEBUG_DP for debug msgs.
changes in V3:
-- Removed changes in dpu_io_util.[ch]
-- Added locking around "is_connected" flag and removed atomic_set()
-- Removed the argument validation checks in all the static functions
except initialization functions and few API calls across msm/dp
files
-- Removed hardcoded values for register reads/writes
-- Removed vreg related generic structures.
-- Added return values where ever necessary.
-- Updated dp_ctrl_on function.
-- Calling the ctrl specific catalog functions directly instead of
function pointers.
-- Added seperate change that adds standard value in drm_dp_helper
file.
-- Added separate change in this list that is used to initialize
displayport in DPU driver.
-- Added change to use drm_dp_get_adjust_request_voltage() function.
Signed-off-by: Chandan Uddaraju <chandanu@xxxxxxxxxxxxxx>
---
[snip]
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index f96e142..29ac7d3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -967,6 +967,9 @@ static void dpu_encoder_virt_mode_set(struct
drm_encoder *drm_enc,
trace_dpu_enc_mode_set(DRMID(drm_enc));
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS &&
priv->dp)
+ msm_dp_display_mode_set(priv->dp, drm_enc, mode,
adj_mode);
+
for better or for worse, DSI and HDMI backends create an internal
drm_bridge object to avoid all of these shunts over from the encoder.
We might be still the only driver to do this, but IMHO it is better
than making each encoder know about each backend, so we might as well
stick with that.
(same goes for the other 'if (drm_enc->encoder_type ==
DRM_MODE_ENCODER_TMDS)'s)
We had the below comments from Sean Paul to remove the bridge object in
patch set 1 of this change.
********** ******************
+static const struct drm_bridge_funcs dp_bridge_ops = {
+ .mode_fixup = dp_bridge_mode_fixup,
+ .pre_enable = dp_bridge_pre_enable,
+ .enable = dp_bridge_enable,
+ .disable = dp_bridge_disable,
+ .post_disable = dp_bridge_post_disable,
+ .mode_set = dp_bridge_mode_set,
+};
I'm not convinced you need any of this. The only advantage a bridge gets
you is
to be involved in modeset. However the only thing you do in modeset is
save the
mode (which is only used in enable). So why not just use the mode from
the
crtc's state when encoder->enable is called?
That allows you to delete all of the bridge stuff here.
+
+int dp_connector_post_init(struct drm_connector *connector, void
*display)
+{
+ struct msm_dp *dp_display = display;
+
+ if (!dp_display)
+ return -EINVAL;
******************************* ****************
thanks
Chandan
BR,
-R
list_for_each_entry(conn_iter, connector_list, head)
if (conn_iter->encoder == drm_enc)
conn = conn_iter;
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
