| Message ID | 20241205-dp_mst-v1-6-f8618d42a99a@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | drm/msm/dp: Add MST support for MSM chipsets | expand |
On Thu, Dec 05, 2024 at 08:31:37PM -0800, Abhinav Kumar wrote: > dp_display caches the current display mode and then passes it onto > the panel to be used for programming the panel params. Remove this > two level passing and directly populated the panel's dp_display_mode > instead. Remove both and use the one from crtc_state? > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 46 ++++++++++++++++--------------------- > 1 file changed, 20 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index 052db80c6a365f53c2c0a37d3b69ea2b627aea1f..4bd85ae754429333aa423c985368344cd03c7752 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -90,7 +90,6 @@ struct msm_dp_display_private { > struct msm_dp_panel *panel; > struct msm_dp_ctrl *ctrl; > > - struct msm_dp_display_mode msm_dp_mode; > struct msm_dp msm_dp_display; > > /* wait for audio signaling */ > @@ -1436,10 +1435,13 @@ bool msm_dp_needs_periph_flush(const struct msm_dp *msm_dp_display, > bool msm_dp_wide_bus_available(const struct msm_dp *msm_dp_display) > { > struct msm_dp_display_private *dp; > + struct msm_dp_panel *dp_panel; > > dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display); > > - if (dp->msm_dp_mode.out_fmt_is_yuv_420) > + dp_panel = dp->panel; > + > + if (dp_panel->msm_dp_mode.out_fmt_is_yuv_420) > return false; > > return dp->wide_bus_supported; > @@ -1501,10 +1503,6 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge, > bool force_link_train = false; > > msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display); > - if (!msm_dp_display->msm_dp_mode.drm_mode.clock) { > - DRM_ERROR("invalid params\n"); > - return; > - } > > if (dp->is_edp) > msm_dp_hpd_plug_handle(msm_dp_display, 0); > @@ -1516,13 +1514,6 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge, > return; > } > > - rc = msm_dp_display_set_mode(dp, &msm_dp_display->msm_dp_mode); > - if (rc) { > - DRM_ERROR("Failed to perform a mode set, rc=%d\n", rc); > - mutex_unlock(&msm_dp_display->event_mutex); > - return; > - } > - > state = msm_dp_display->hpd_state; > > if (state == ST_CONNECTED && !dp->power_on) { > @@ -1599,37 +1590,40 @@ void msm_dp_bridge_mode_set(struct drm_bridge *drm_bridge, > struct msm_dp *dp = msm_dp_bridge->msm_dp_display; > struct msm_dp_display_private *msm_dp_display; > struct msm_dp_panel *msm_dp_panel; > + struct msm_dp_display_mode msm_dp_mode; No need to allocate it on stack just to copy it later on. Please write the data directly to a proper location from the beginning. > > msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display); > msm_dp_panel = msm_dp_display->panel; > > - memset(&msm_dp_display->msm_dp_mode, 0x0, sizeof(struct msm_dp_display_mode)); > + memset(&msm_dp_mode, 0x0, sizeof(struct msm_dp_display_mode)); > > if (msm_dp_display_check_video_test(dp)) > - msm_dp_display->msm_dp_mode.bpp = msm_dp_display_get_test_bpp(dp); > + msm_dp_mode.bpp = msm_dp_display_get_test_bpp(dp); > else /* Default num_components per pixel = 3 */ > - msm_dp_display->msm_dp_mode.bpp = dp->connector->display_info.bpc * 3; > + msm_dp_mode.bpp = dp->connector->display_info.bpc * 3; > > - if (!msm_dp_display->msm_dp_mode.bpp) > - msm_dp_display->msm_dp_mode.bpp = 24; /* Default bpp */ > + if (!msm_dp_mode.bpp) > + msm_dp_mode.bpp = 24; /* Default bpp */ The msm_dp_mode.bpp gets rewritten by msm_dp_panel_init_panel_info() after being set here. Is this code part redundant? > > - drm_mode_copy(&msm_dp_display->msm_dp_mode.drm_mode, adjusted_mode); > + drm_mode_copy(&msm_dp_mode.drm_mode, adjusted_mode); > > - msm_dp_display->msm_dp_mode.v_active_low = > - !!(msm_dp_display->msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NVSYNC); > + msm_dp_mode.v_active_low = > + !!(msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NVSYNC); > > - msm_dp_display->msm_dp_mode.h_active_low = > - !!(msm_dp_display->msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); > + msm_dp_mode.h_active_low = > + !!(msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); > > - msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 = > + msm_dp_mode.out_fmt_is_yuv_420 = > drm_mode_is_420_only(&dp->connector->display_info, adjusted_mode) && > msm_dp_panel->vsc_sdp_supported; > > + msm_dp_display_set_mode(dp, &msm_dp_mode); > + > /* populate wide_bus_support to different layers */ > msm_dp_display->ctrl->wide_bus_en = > - msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; > + msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; > msm_dp_display->catalog->wide_bus_en = > - msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; > + msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; > } > > void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge) > > -- > 2.34.1 >
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 052db80c6a365f53c2c0a37d3b69ea2b627aea1f..4bd85ae754429333aa423c985368344cd03c7752 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -90,7 +90,6 @@ struct msm_dp_display_private { struct msm_dp_panel *panel; struct msm_dp_ctrl *ctrl; - struct msm_dp_display_mode msm_dp_mode; struct msm_dp msm_dp_display; /* wait for audio signaling */ @@ -1436,10 +1435,13 @@ bool msm_dp_needs_periph_flush(const struct msm_dp *msm_dp_display, bool msm_dp_wide_bus_available(const struct msm_dp *msm_dp_display) { struct msm_dp_display_private *dp; + struct msm_dp_panel *dp_panel; dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display); - if (dp->msm_dp_mode.out_fmt_is_yuv_420) + dp_panel = dp->panel; + + if (dp_panel->msm_dp_mode.out_fmt_is_yuv_420) return false; return dp->wide_bus_supported; @@ -1501,10 +1503,6 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge, bool force_link_train = false; msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display); - if (!msm_dp_display->msm_dp_mode.drm_mode.clock) { - DRM_ERROR("invalid params\n"); - return; - } if (dp->is_edp) msm_dp_hpd_plug_handle(msm_dp_display, 0); @@ -1516,13 +1514,6 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge, return; } - rc = msm_dp_display_set_mode(dp, &msm_dp_display->msm_dp_mode); - if (rc) { - DRM_ERROR("Failed to perform a mode set, rc=%d\n", rc); - mutex_unlock(&msm_dp_display->event_mutex); - return; - } - state = msm_dp_display->hpd_state; if (state == ST_CONNECTED && !dp->power_on) { @@ -1599,37 +1590,40 @@ void msm_dp_bridge_mode_set(struct drm_bridge *drm_bridge, struct msm_dp *dp = msm_dp_bridge->msm_dp_display; struct msm_dp_display_private *msm_dp_display; struct msm_dp_panel *msm_dp_panel; + struct msm_dp_display_mode msm_dp_mode; msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display); msm_dp_panel = msm_dp_display->panel; - memset(&msm_dp_display->msm_dp_mode, 0x0, sizeof(struct msm_dp_display_mode)); + memset(&msm_dp_mode, 0x0, sizeof(struct msm_dp_display_mode)); if (msm_dp_display_check_video_test(dp)) - msm_dp_display->msm_dp_mode.bpp = msm_dp_display_get_test_bpp(dp); + msm_dp_mode.bpp = msm_dp_display_get_test_bpp(dp); else /* Default num_components per pixel = 3 */ - msm_dp_display->msm_dp_mode.bpp = dp->connector->display_info.bpc * 3; + msm_dp_mode.bpp = dp->connector->display_info.bpc * 3; - if (!msm_dp_display->msm_dp_mode.bpp) - msm_dp_display->msm_dp_mode.bpp = 24; /* Default bpp */ + if (!msm_dp_mode.bpp) + msm_dp_mode.bpp = 24; /* Default bpp */ - drm_mode_copy(&msm_dp_display->msm_dp_mode.drm_mode, adjusted_mode); + drm_mode_copy(&msm_dp_mode.drm_mode, adjusted_mode); - msm_dp_display->msm_dp_mode.v_active_low = - !!(msm_dp_display->msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NVSYNC); + msm_dp_mode.v_active_low = + !!(msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NVSYNC); - msm_dp_display->msm_dp_mode.h_active_low = - !!(msm_dp_display->msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); + msm_dp_mode.h_active_low = + !!(msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); - msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 = + msm_dp_mode.out_fmt_is_yuv_420 = drm_mode_is_420_only(&dp->connector->display_info, adjusted_mode) && msm_dp_panel->vsc_sdp_supported; + msm_dp_display_set_mode(dp, &msm_dp_mode); + /* populate wide_bus_support to different layers */ msm_dp_display->ctrl->wide_bus_en = - msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; + msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; msm_dp_display->catalog->wide_bus_en = - msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; + msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported; } void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge)
dp_display caches the current display mode and then passes it onto the panel to be used for programming the panel params. Remove this two level passing and directly populated the panel's dp_display_mode instead. Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/dp/dp_display.c | 46 ++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 26 deletions(-)