From 79ebc86c41959113009e3d9b17dc2d4547512ac9 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Mon, 16 Apr 2018 19:05:30 -0700 Subject: drm/msm/dsi: check return value for video done waits Check for the return value of wait for video done waits and print appropriate error message. Signed-off-by: Abhinav Kumar Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/dsi/dsi_host.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 7a03a9489708..93a3cdde72ef 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -986,13 +986,19 @@ static void dsi_set_tx_power_mode(int mode, struct msm_dsi_host *msm_host) static void dsi_wait4video_done(struct msm_dsi_host *msm_host) { + u32 ret = 0; + struct device *dev = &msm_host->pdev->dev; + dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1); reinit_completion(&msm_host->video_comp); - wait_for_completion_timeout(&msm_host->video_comp, + ret = wait_for_completion_timeout(&msm_host->video_comp, msecs_to_jiffies(70)); + if (ret <= 0) + dev_err(dev, "wait for video done timed out\n"); + dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0); } -- cgit v1.2.3-59-g8ed1b From 9c5638d78df23e3fc5a6f15972f6fc20939ffe8b Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Tue, 17 Apr 2018 13:50:18 -0700 Subject: drm/msm/dsi: check video mode engine status before waiting Make sure the video mode engine is on before waiting for the video done interrupt. Changes in v4: - Move setting enabled to false earlier Changes in v3: - Move the return value check to another patch Changes in v2: - Replace pr_err with dev_err - Changed error message Signed-off-by: Abhinav Kumar Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/dsi/dsi_host.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 93a3cdde72ef..2f770386ab4a 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -173,6 +173,7 @@ struct msm_dsi_host { bool registered; bool power_on; + bool enabled; int irq; }; @@ -1007,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct msm_dsi_host *msm_host) if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO)) return; - if (msm_host->power_on) { + if (msm_host->power_on && msm_host->enabled) { dsi_wait4video_done(msm_host); /* delay 4 ms to skip BLLP */ usleep_range(2000, 4000); @@ -2209,7 +2210,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host) * pm_runtime_put_autosuspend(&msm_host->pdev->dev); * } */ - + msm_host->enabled = true; return 0; } @@ -2217,6 +2218,7 @@ int msm_dsi_host_disable(struct mipi_dsi_host *host) { struct msm_dsi_host *msm_host = to_msm_dsi_host(host); + msm_host->enabled = false; dsi_op_mode_config(msm_host, !!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO), false); -- cgit v1.2.3-59-g8ed1b From f7b6bf20e26384c615a6f5d39eee148c36ed4003 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Tue, 17 Apr 2018 13:50:19 -0700 Subject: drm/msm/dsi: implement auto PHY timing calculator for 10nm PHY Currently the DSI PHY timings are hard-coded for a specific panel for the 10nm PHY. Replace this with the auto PHY timing calculator which can calculate the PHY timings for any panel. Changes in v4: - None Changes in v3: - None Changes in v2: - None Reviewed-by: Sean Paul Reviewed-by: Archit Taneja Signed-off-by: Abhinav Kumar Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 109 +++++++++++++++++++++++++++++ drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 2 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 28 -------- 3 files changed, 111 insertions(+), 28 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c index 8e9d5c255820..9a9fa0c75a13 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c @@ -265,6 +265,115 @@ int msm_dsi_dphy_timing_calc_v2(struct msm_dsi_dphy_timing *timing, return 0; } +int msm_dsi_dphy_timing_calc_v3(struct msm_dsi_dphy_timing *timing, + struct msm_dsi_phy_clk_request *clk_req) +{ + const unsigned long bit_rate = clk_req->bitclk_rate; + const unsigned long esc_rate = clk_req->escclk_rate; + s32 ui, ui_x8, lpx; + s32 tmax, tmin; + s32 pcnt0 = 50; + s32 pcnt1 = 50; + s32 pcnt2 = 10; + s32 pcnt3 = 30; + s32 pcnt4 = 10; + s32 pcnt5 = 2; + s32 coeff = 1000; /* Precision, should avoid overflow */ + s32 hb_en, hb_en_ckln; + s32 temp; + + if (!bit_rate || !esc_rate) + return -EINVAL; + + timing->hs_halfbyte_en = 0; + hb_en = 0; + timing->hs_halfbyte_en_ckln = 0; + hb_en_ckln = 0; + + ui = mult_frac(NSEC_PER_MSEC, coeff, bit_rate / 1000); + ui_x8 = ui << 3; + lpx = mult_frac(NSEC_PER_MSEC, coeff, esc_rate / 1000); + + temp = S_DIV_ROUND_UP(38 * coeff, ui_x8); + tmin = max_t(s32, temp, 0); + temp = (95 * coeff) / ui_x8; + tmax = max_t(s32, temp, 0); + timing->clk_prepare = linear_inter(tmax, tmin, pcnt0, 0, false); + + temp = 300 * coeff - (timing->clk_prepare << 3) * ui; + tmin = S_DIV_ROUND_UP(temp, ui_x8) - 1; + tmax = (tmin > 255) ? 511 : 255; + timing->clk_zero = linear_inter(tmax, tmin, pcnt5, 0, false); + + tmin = DIV_ROUND_UP(60 * coeff + 3 * ui, ui_x8); + temp = 105 * coeff + 12 * ui - 20 * coeff; + tmax = (temp + 3 * ui) / ui_x8; + timing->clk_trail = linear_inter(tmax, tmin, pcnt3, 0, false); + + temp = S_DIV_ROUND_UP(40 * coeff + 4 * ui, ui_x8); + tmin = max_t(s32, temp, 0); + temp = (85 * coeff + 6 * ui) / ui_x8; + tmax = max_t(s32, temp, 0); + timing->hs_prepare = linear_inter(tmax, tmin, pcnt1, 0, false); + + temp = 145 * coeff + 10 * ui - (timing->hs_prepare << 3) * ui; + tmin = S_DIV_ROUND_UP(temp, ui_x8) - 1; + tmax = 255; + timing->hs_zero = linear_inter(tmax, tmin, pcnt4, 0, false); + + tmin = DIV_ROUND_UP(60 * coeff + 4 * ui, ui_x8) - 1; + temp = 105 * coeff + 12 * ui - 20 * coeff; + tmax = (temp / ui_x8) - 1; + timing->hs_trail = linear_inter(tmax, tmin, pcnt3, 0, false); + + temp = 50 * coeff + ((hb_en << 2) - 8) * ui; + timing->hs_rqst = S_DIV_ROUND_UP(temp, ui_x8); + + tmin = DIV_ROUND_UP(100 * coeff, ui_x8) - 1; + tmax = 255; + timing->hs_exit = linear_inter(tmax, tmin, pcnt2, 0, false); + + temp = 50 * coeff + ((hb_en_ckln << 2) - 8) * ui; + timing->hs_rqst_ckln = S_DIV_ROUND_UP(temp, ui_x8); + + temp = 60 * coeff + 52 * ui - 43 * ui; + tmin = DIV_ROUND_UP(temp, ui_x8) - 1; + tmax = 63; + timing->shared_timings.clk_post = + linear_inter(tmax, tmin, pcnt2, 0, false); + + temp = 8 * ui + (timing->clk_prepare << 3) * ui; + temp += (((timing->clk_zero + 3) << 3) + 11) * ui; + temp += hb_en_ckln ? (((timing->hs_rqst_ckln << 3) + 4) * ui) : + (((timing->hs_rqst_ckln << 3) + 8) * ui); + tmin = S_DIV_ROUND_UP(temp, ui_x8) - 1; + tmax = 63; + if (tmin > tmax) { + temp = linear_inter(tmax << 1, tmin, pcnt2, 0, false); + timing->shared_timings.clk_pre = temp >> 1; + timing->shared_timings.clk_pre_inc_by_2 = 1; + } else { + timing->shared_timings.clk_pre = + linear_inter(tmax, tmin, pcnt2, 0, false); + timing->shared_timings.clk_pre_inc_by_2 = 0; + } + + timing->ta_go = 3; + timing->ta_sure = 0; + timing->ta_get = 4; + + DBG("%d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d", + timing->shared_timings.clk_pre, timing->shared_timings.clk_post, + timing->shared_timings.clk_pre_inc_by_2, timing->clk_zero, + timing->clk_trail, timing->clk_prepare, timing->hs_exit, + timing->hs_zero, timing->hs_prepare, timing->hs_trail, + timing->hs_rqst, timing->hs_rqst_ckln, timing->hs_halfbyte_en, + timing->hs_halfbyte_en_ckln, timing->hs_prep_dly, + timing->hs_prep_dly_ckln); + + return 0; +} + void msm_dsi_phy_set_src_pll(struct msm_dsi_phy *phy, int pll_id, u32 reg, u32 bit_mask) { diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h index c56268cbdb3d..a24ab80994a3 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h @@ -101,6 +101,8 @@ int msm_dsi_dphy_timing_calc(struct msm_dsi_dphy_timing *timing, struct msm_dsi_phy_clk_request *clk_req); int msm_dsi_dphy_timing_calc_v2(struct msm_dsi_dphy_timing *timing, struct msm_dsi_phy_clk_request *clk_req); +int msm_dsi_dphy_timing_calc_v3(struct msm_dsi_dphy_timing *timing, + struct msm_dsi_phy_clk_request *clk_req); void msm_dsi_phy_set_src_pll(struct msm_dsi_phy *phy, int pll_id, u32 reg, u32 bit_mask); int msm_dsi_phy_init_common(struct msm_dsi_phy *phy); diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c index 0af951aaeea1..b3fffc8dbb2a 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c @@ -79,34 +79,6 @@ static void dsi_phy_hw_v3_0_lane_settings(struct msm_dsi_phy *phy) dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3), 0x04); } -static int msm_dsi_dphy_timing_calc_v3(struct msm_dsi_dphy_timing *timing, - struct msm_dsi_phy_clk_request *clk_req) -{ - /* - * TODO: These params need to be computed, they're currently hardcoded - * for a 1440x2560@60Hz panel with a byteclk of 100.618 Mhz, and a - * default escape clock of 19.2 Mhz. - */ - - timing->hs_halfbyte_en = 0; - timing->clk_zero = 0x1c; - timing->clk_prepare = 0x07; - timing->clk_trail = 0x07; - timing->hs_exit = 0x23; - timing->hs_zero = 0x21; - timing->hs_prepare = 0x07; - timing->hs_trail = 0x07; - timing->hs_rqst = 0x05; - timing->ta_sure = 0x00; - timing->ta_go = 0x03; - timing->ta_get = 0x04; - - timing->shared_timings.clk_pre = 0x2d; - timing->shared_timings.clk_post = 0x0d; - - return 0; -} - static int dsi_10nm_phy_enable(struct msm_dsi_phy *phy, int src_pll_id, struct msm_dsi_phy_clk_request *clk_req) { -- cgit v1.2.3-59-g8ed1b From 20387275142422103c3734238685cbee6ac0dffa Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Wed, 28 Feb 2018 14:19:00 -0500 Subject: drm/msm: Mark the crtc->state->event consumed Don't leave the event != NULL once it's consumed, this is used a signal to the atomic helpers that the event will be handled by the driver. Changes in v2: - None Changes in v3: - Rebased on Archit's private_obj set Changes in v4: - None Cc: Jeykumar Sankaran Reviewed-by: Archit Taneja Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 + 2 files changed, 2 insertions(+) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c index 6e5e1aa54ce1..b001699297c4 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c @@ -351,6 +351,7 @@ static void mdp4_crtc_atomic_flush(struct drm_crtc *crtc, spin_lock_irqsave(&dev->event_lock, flags); mdp4_crtc->event = crtc->state->event; + crtc->state->event = NULL; spin_unlock_irqrestore(&dev->event_lock, flags); blend_setup(crtc); diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index 9893e43ba6c5..76b96081916f 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -708,6 +708,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, spin_lock_irqsave(&dev->event_lock, flags); mdp5_crtc->event = crtc->state->event; + crtc->state->event = NULL; spin_unlock_irqrestore(&dev->event_lock, flags); /* -- cgit v1.2.3-59-g8ed1b From 4e4902324a9b46a9111d5e514301e154f938238a Mon Sep 17 00:00:00 2001 From: Jeykumar Sankaran Date: Tue, 13 Feb 2018 12:42:44 -0500 Subject: drm/msm: Add modifier to mdp_get_format arguments This change plumbs the new fb modifier through the various mdp/disp get_format hooks. Signed-off-by: Jeykumar Sankaran [seanpaul pimped out commit message a bit] Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/mdp_format.c | 3 ++- drivers/gpu/drm/msm/disp/mdp_kms.h | 2 +- drivers/gpu/drm/msm/msm_fb.c | 3 ++- drivers/gpu/drm/msm/msm_kms.h | 5 ++++- 4 files changed, 9 insertions(+), 4 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/disp/mdp_format.c b/drivers/gpu/drm/msm/disp/mdp_format.c index b4a8aa4490ee..005760bee708 100644 --- a/drivers/gpu/drm/msm/disp/mdp_format.c +++ b/drivers/gpu/drm/msm/disp/mdp_format.c @@ -171,7 +171,8 @@ uint32_t mdp_get_formats(uint32_t *pixel_formats, uint32_t max_formats, return i; } -const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format) +const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format, + uint64_t modifier) { int i; for (i = 0; i < ARRAY_SIZE(formats); i++) { diff --git a/drivers/gpu/drm/msm/disp/mdp_kms.h b/drivers/gpu/drm/msm/disp/mdp_kms.h index 1185487e7e5e..4fa8dbe4e165 100644 --- a/drivers/gpu/drm/msm/disp/mdp_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp_kms.h @@ -98,7 +98,7 @@ struct mdp_format { #define MDP_FORMAT_IS_YUV(mdp_format) ((mdp_format)->is_yuv) uint32_t mdp_get_formats(uint32_t *formats, uint32_t max_formats, bool rgb_only); -const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format); +const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format, uint64_t modifier); /* MDP capabilities */ #define MDP_CAP_SMP BIT(0) /* Shared Memory Pool */ diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c index 0e0c87252ab0..7a16242bf8bf 100644 --- a/drivers/gpu/drm/msm/msm_fb.c +++ b/drivers/gpu/drm/msm/msm_fb.c @@ -183,7 +183,8 @@ static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev, hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format); vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format); - format = kms->funcs->get_format(kms, mode_cmd->pixel_format); + format = kms->funcs->get_format(kms, mode_cmd->pixel_format, + mode_cmd->modifier[0]); if (!format) { dev_err(dev->dev, "unsupported pixel format: %4.4s\n", (char *)&mode_cmd->pixel_format); diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 17d5824417ad..aaa329dc020e 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -48,8 +48,11 @@ struct msm_kms_funcs { /* functions to wait for atomic commit completed on each CRTC */ void (*wait_for_crtc_commit_done)(struct msm_kms *kms, struct drm_crtc *crtc); + /* get msm_format w/ optional format modifiers from drm_mode_fb_cmd2 */ + const struct msm_format *(*get_format)(struct msm_kms *kms, + const uint32_t format, + const uint64_t modifiers); /* misc: */ - const struct msm_format *(*get_format)(struct msm_kms *kms, uint32_t format); long (*round_pixclk)(struct msm_kms *kms, unsigned long rate, struct drm_encoder *encoder); int (*set_split_display)(struct msm_kms *kms, -- cgit v1.2.3-59-g8ed1b From 03c94d60261c28689465a16086eb290ab3012aa3 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Tue, 3 Apr 2018 23:38:45 +0100 Subject: drm/msm: Fix possible null dereference on failure of get_pages() Commit 62e3a3e342af changed get_pages() to initialise msm_gem_object::pages before trying to initialise msm_gem_object::sgt, so that put_pages() would properly clean up pages in the failure case. However, this means that put_pages() now needs to check that msm_gem_object::sgt is not null before trying to clean it up, and this check was only applied to part of the cleanup code. Move it all into the conditional block. (Strictly speaking we don't need to make the kfree() conditional, but since we can't avoid checking for null ourselves we may as well do so.) Fixes: 62e3a3e342af ("drm/msm: fix leak in failed get_pages") Signed-off-by: Ben Hutchings Reviewed-by: Jordan Crouse Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 95196479f651..f583bb4222f9 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -132,17 +132,19 @@ static void put_pages(struct drm_gem_object *obj) struct msm_gem_object *msm_obj = to_msm_bo(obj); if (msm_obj->pages) { - /* For non-cached buffers, ensure the new pages are clean - * because display controller, GPU, etc. are not coherent: - */ - if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED)) - dma_unmap_sg(obj->dev->dev, msm_obj->sgt->sgl, - msm_obj->sgt->nents, DMA_BIDIRECTIONAL); + if (msm_obj->sgt) { + /* For non-cached buffers, ensure the new + * pages are clean because display controller, + * GPU, etc. are not coherent: + */ + if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED)) + dma_unmap_sg(obj->dev->dev, msm_obj->sgt->sgl, + msm_obj->sgt->nents, + DMA_BIDIRECTIONAL); - if (msm_obj->sgt) sg_free_table(msm_obj->sgt); - - kfree(msm_obj->sgt); + kfree(msm_obj->sgt); + } if (use_pages(obj)) drm_gem_put_pages(obj, msm_obj->pages, true, false); -- cgit v1.2.3-59-g8ed1b From cf606fe3300cbd3db44785174934e61e5a2ffc37 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 19 Mar 2018 22:26:32 +0100 Subject: drm/msm/dsi: use correct enum in dsi_get_cmd_fmt The function dsi_get_cmd_fmt returns enum dsi_cmd_dst_format, use the correct enum value also for MIPI_DSI_FMT_RGB666/_PACKED. This has been discovered using clang: drivers/gpu/drm/msm/dsi/dsi_host.c:743:35: warning: implicit conversion from enumeration type 'enum dsi_vid_dst_format' to different enumeration type 'enum dsi_cmd_dst_format' [-Wenum-conversion] case MIPI_DSI_FMT_RGB666: return VID_DST_FORMAT_RGB666; ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Stefan Agner Reviewed-by: Archit Taneja Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 2f770386ab4a..8baba30d6c65 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -776,7 +776,7 @@ static inline enum dsi_cmd_dst_format dsi_get_cmd_fmt( switch (mipi_fmt) { case MIPI_DSI_FMT_RGB888: return CMD_DST_FORMAT_RGB888; case MIPI_DSI_FMT_RGB666_PACKED: - case MIPI_DSI_FMT_RGB666: return VID_DST_FORMAT_RGB666; + case MIPI_DSI_FMT_RGB666: return CMD_DST_FORMAT_RGB666; case MIPI_DSI_FMT_RGB565: return CMD_DST_FORMAT_RGB565; default: return CMD_DST_FORMAT_RGB888; } -- cgit v1.2.3-59-g8ed1b From 641be142bfc241b3e1ce7ecf3f04ef03facfe81e Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Wed, 28 Mar 2018 17:22:16 +0100 Subject: drm/msm: don't deref error pointer in the msm_fbdev_create error path Currently the error pointer returned by msm_alloc_stolen_fb gets passed to drm_framebuffer_remove. The latter handles only NULL pointers, thus a nasty crash will occur. Drop the unnecessary fail label and the associated checks - both err and fb will be set at this stage. Cc: Rob Clark Cc: linux-arm-msm@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: freedreno@lists.freedesktop.org Signed-off-by: Emil Velikov Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_fbdev.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c index c178563fcd4d..456622b46335 100644 --- a/drivers/gpu/drm/msm/msm_fbdev.c +++ b/drivers/gpu/drm/msm/msm_fbdev.c @@ -92,8 +92,7 @@ static int msm_fbdev_create(struct drm_fb_helper *helper, if (IS_ERR(fb)) { dev_err(dev->dev, "failed to allocate fb\n"); - ret = PTR_ERR(fb); - goto fail; + return PTR_ERR(fb); } bo = msm_framebuffer_bo(fb, 0); @@ -151,13 +150,7 @@ static int msm_fbdev_create(struct drm_fb_helper *helper, fail_unlock: mutex_unlock(&dev->struct_mutex); -fail: - - if (ret) { - if (fb) - drm_framebuffer_remove(fb); - } - + drm_framebuffer_remove(fb); return ret; } -- cgit v1.2.3-59-g8ed1b From 3c9620cdff9e94bc1b8dd688612f0ff46da78e0f Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Thu, 3 May 2018 14:00:55 +0200 Subject: drm/msm: remove unbalanced mutex unlock This regression stems from 0e08270a1f01 ("drm/msm: Separate locking of buffer resources from struct_mutex"). Signed-off-by: Daniel Mack Cc: Sushmita Susheelendra Cc: Rob Clark Fixes: 0e08270a1f01 ("drm/msm: Separate locking of buffer resources from struct_mutex") Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/dsi/dsi_host.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 8baba30d6c65..c4d23d4c3631 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1036,7 +1036,6 @@ static int dsi_tx_buf_alloc(struct msm_dsi_host *msm_host, int size) ret = msm_gem_get_iova(msm_host->tx_gem_obj, priv->kms->aspace, &iova); - mutex_unlock(&dev->struct_mutex); if (ret) { pr_err("%s: failed to get iova, %d\n", __func__, ret); return ret; -- cgit v1.2.3-59-g8ed1b From acb1acdb69c85375ee80777336dff4df2682da11 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Thu, 3 May 2018 14:00:56 +0200 Subject: drm/msm: use correct aspace pointer in msm_gem_put_iova() Even though msm_gem_put_iova() is currently a NOP function, the caller should pass in the address space pointer it used to obtain the object. Other call sites were changed in 8bdcd949bbe7e ("drm/msm: pass address-space to _get_iova() and friends"), but this one seems to have been forgotten. Signed-off-by: Daniel Mack Cc: Rob Clark Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index c4d23d4c3631..b916f464f4ec 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1066,9 +1066,10 @@ static int dsi_tx_buf_alloc(struct msm_dsi_host *msm_host, int size) static void dsi_tx_buf_free(struct msm_dsi_host *msm_host) { struct drm_device *dev = msm_host->dev; + struct msm_drm_private *priv = dev->dev_private; if (msm_host->tx_gem_obj) { - msm_gem_put_iova(msm_host->tx_gem_obj, 0); + msm_gem_put_iova(msm_host->tx_gem_obj, priv->kms->aspace); drm_gem_object_put_unlocked(msm_host->tx_gem_obj); msm_host->tx_gem_obj = NULL; } -- cgit v1.2.3-59-g8ed1b From 8d58ef346f30cbbeb9213a7eb90c832abf903fa0 Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Wed, 21 Feb 2018 09:37:22 -0500 Subject: drm/msm/mdp5: Add global state as a private atomic object Global shared resources (hwpipes, hwmixers and SMP) for MDP5 are implemented as a part of atomic state by subclassing drm_atomic_state. The preferred approach is to use the drm_private_obj infrastructure available in the atomic core. mdp5_global_state is introduced as a drm atomic private object. The two funcs mdp5_get_global_state() and mdp5_get_existing_global_state() are the two variants that will be used to access mdp5_global_state. This will replace the existing mdp5_state struct (which subclasses drm_atomic_state) and the funcs around it. These will be removed later once we mdp5_global_state is put to use everywhere. Changes in v3: - Added glob_state_lock instead of pushing it into the core - Added to the msm atomic helper patch set Changes in v4: - None Signed-off-by: Archit Taneja Signed-off-by: Rob Clark Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 87 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 25 +++++++++ 2 files changed, 112 insertions(+) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 6d8e3a9a6fc0..fcbdef385a8a 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -106,6 +106,86 @@ static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state) swap(to_kms_state(state)->state, mdp5_kms->state); } +/* Global/shared object state funcs */ + +/* + * This is a helper that returns the private state currently in operation. + * Note that this would return the "old_state" if called in the atomic check + * path, and the "new_state" after the atomic swap has been done. + */ +struct mdp5_global_state * +mdp5_get_existing_global_state(struct mdp5_kms *mdp5_kms) +{ + return to_mdp5_global_state(mdp5_kms->glob_state.state); +} + +/* + * This acquires the modeset lock set aside for global state, creates + * a new duplicated private object state. + */ +struct mdp5_global_state *mdp5_get_global_state(struct drm_atomic_state *s) +{ + struct msm_drm_private *priv = s->dev->dev_private; + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms)); + struct drm_private_state *priv_state; + int ret; + + ret = drm_modeset_lock(&mdp5_kms->glob_state_lock, s->acquire_ctx); + if (ret) + return ERR_PTR(ret); + + priv_state = drm_atomic_get_private_obj_state(s, &mdp5_kms->glob_state); + if (IS_ERR(priv_state)) + return ERR_CAST(priv_state); + + return to_mdp5_global_state(priv_state); +} + +static struct drm_private_state * +mdp5_global_duplicate_state(struct drm_private_obj *obj) +{ + struct mdp5_global_state *state; + + state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base); + + return &state->base; +} + +static void mdp5_global_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct mdp5_global_state *mdp5_state = to_mdp5_global_state(state); + + kfree(mdp5_state); +} + +static const struct drm_private_state_funcs mdp5_global_state_funcs = { + .atomic_duplicate_state = mdp5_global_duplicate_state, + .atomic_destroy_state = mdp5_global_destroy_state, +}; + +static int mdp5_global_obj_init(struct mdp5_kms *mdp5_kms) +{ + struct mdp5_global_state *state; + + drm_modeset_lock_init(&mdp5_kms->glob_state_lock); + + state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return -ENOMEM; + + state->mdp5_kms = mdp5_kms; + + drm_atomic_private_obj_init(&mdp5_kms->glob_state, + &state->base, + &mdp5_global_state_funcs); + return 0; +} + static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state) { struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); @@ -727,6 +807,9 @@ static void mdp5_destroy(struct platform_device *pdev) if (mdp5_kms->rpm_enabled) pm_runtime_disable(&pdev->dev); + drm_atomic_private_obj_fini(&mdp5_kms->glob_state); + drm_modeset_lock_fini(&mdp5_kms->glob_state_lock); + kfree(mdp5_kms->state); } @@ -887,6 +970,10 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev) goto fail; } + ret = mdp5_global_obj_init(mdp5_kms); + if (ret) + goto fail; + mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys", "MDP5"); if (IS_ERR(mdp5_kms->mmio)) { ret = PTR_ERR(mdp5_kms->mmio); diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h index 425a03d213e5..76f0ddfca322 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h @@ -55,6 +55,13 @@ struct mdp5_kms { struct mdp5_state *state; struct drm_modeset_lock state_lock; + /* + * Global private object state, Do not access directly, use + * mdp5_global_get_state() + */ + struct drm_modeset_lock glob_state_lock; + struct drm_private_obj glob_state; + struct mdp5_smp *smp; struct mdp5_ctl_manager *ctlm; @@ -95,6 +102,24 @@ struct mdp5_state { struct mdp5_state *__must_check mdp5_get_state(struct drm_atomic_state *s); +/* Global private object state for tracking resources that are shared across + * multiple kms objects (planes/crtcs/etc). + */ +#define to_mdp5_global_state(x) container_of(x, struct mdp5_global_state, base) +struct mdp5_global_state { + struct drm_private_state base; + + struct drm_atomic_state *state; + struct mdp5_kms *mdp5_kms; + + struct mdp5_hw_pipe_state hwpipe; + struct mdp5_hw_mixer_state hwmixer; + struct mdp5_smp_state smp; +}; + +struct mdp5_global_state * mdp5_get_existing_global_state(struct mdp5_kms *mdp5_kms); +struct mdp5_global_state *__must_check mdp5_get_global_state(struct drm_atomic_state *s); + /* Atomic plane state. Subclasses the base drm_plane_state in order to * track assigned hwpipe and hw specific state. */ -- cgit v1.2.3-59-g8ed1b From 7907a0d77cb461f58045763c205a5830be72e97c Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Wed, 21 Feb 2018 09:37:23 -0500 Subject: drm/msm/mdp5: Use the new private_obj state This replaces the usage of the subclassed atomic state (mdp5_state) with a private_obj state embedded within drm_atomic_state. The latter method is the preferred approach, since it's simpler to implement and less prone to errors. The new API replaces the older and equivalent mdp5_state usage in the following pattern: - References to "mdp5_kms->state" (i.e, the old/existing state) is replaced with mdp5_get_existing_global_state(). In the atomic_check path, this should be called with the glob_state_lock drm_modeset_lock alredy taken. - References to "mdp5_get_state()" are replaced with mdp5_get_global_state(). This acquires glob_state_lock and uses drm_atomic_get_private_obj_state() to create a new duplicated state. Changes in v3: - Acquire glob_state_lock in mdp5_smp.c - Added to the msm atomic helper patch set Changes in v4: - None Signed-off-by: Archit Taneja Signed-off-by: Rob Clark Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 10 ++++++++-- drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 ++++++------ drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 20 +++++++++++--------- drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 17 ++++++++++++----- 4 files changed, 37 insertions(+), 22 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index fcbdef385a8a..6ada098dba0b 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -190,20 +190,26 @@ static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *st { struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); struct device *dev = &mdp5_kms->pdev->dev; + struct mdp5_global_state *global_state; + + global_state = mdp5_get_existing_global_state(mdp5_kms); pm_runtime_get_sync(dev); if (mdp5_kms->smp) - mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_kms->state->smp); + mdp5_smp_prepare_commit(mdp5_kms->smp, &global_state->smp); } static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state) { struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); struct device *dev = &mdp5_kms->pdev->dev; + struct mdp5_global_state *global_state; + + global_state = mdp5_get_existing_global_state(mdp5_kms); if (mdp5_kms->smp) - mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_kms->state->smp); + mdp5_smp_complete_commit(mdp5_kms->smp, &global_state->smp); pm_runtime_put_sync(dev); } diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c index 8a00991f03c7..113e6b569562 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c @@ -52,14 +52,14 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc, { struct msm_drm_private *priv = s->dev->dev_private; struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms)); - struct mdp5_state *state = mdp5_get_state(s); + struct mdp5_global_state *global_state = mdp5_get_global_state(s); struct mdp5_hw_mixer_state *new_state; int i; - if (IS_ERR(state)) - return PTR_ERR(state); + if (IS_ERR(global_state)) + return PTR_ERR(global_state); - new_state = &state->hwmixer; + new_state = &global_state->hwmixer; for (i = 0; i < mdp5_kms->num_hwmixers; i++) { struct mdp5_hw_mixer *cur = mdp5_kms->hwmixers[i]; @@ -129,8 +129,8 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc, void mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer *mixer) { - struct mdp5_state *state = mdp5_get_state(s); - struct mdp5_hw_mixer_state *new_state = &state->hwmixer; + struct mdp5_global_state *global_state = mdp5_get_global_state(s); + struct mdp5_hw_mixer_state *new_state = &global_state->hwmixer; if (!mixer) return; diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c index ff52c49095f9..1ef26bc63163 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c @@ -24,17 +24,19 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane, { struct msm_drm_private *priv = s->dev->dev_private; struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms)); - struct mdp5_state *state; + struct mdp5_global_state *new_global_state, *old_global_state; struct mdp5_hw_pipe_state *old_state, *new_state; int i, j; - state = mdp5_get_state(s); - if (IS_ERR(state)) - return PTR_ERR(state); + new_global_state = mdp5_get_global_state(s); + if (IS_ERR(new_global_state)) + return PTR_ERR(new_global_state); - /* grab old_state after mdp5_get_state(), since now we hold lock: */ - old_state = &mdp5_kms->state->hwpipe; - new_state = &state->hwpipe; + /* grab old_state after mdp5_get_global_state(), since now we hold lock: */ + old_global_state = mdp5_get_existing_global_state(mdp5_kms); + + old_state = &old_global_state->hwpipe; + new_state = &new_global_state->hwpipe; for (i = 0; i < mdp5_kms->num_hwpipes; i++) { struct mdp5_hw_pipe *cur = mdp5_kms->hwpipes[i]; @@ -107,7 +109,7 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane, WARN_ON(r_hwpipe); DBG("%s: alloc SMP blocks", (*hwpipe)->name); - ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp, + ret = mdp5_smp_assign(mdp5_kms->smp, &new_global_state->smp, (*hwpipe)->pipe, blkcfg); if (ret) return -ENOMEM; @@ -132,7 +134,7 @@ void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe) { struct msm_drm_private *priv = s->dev->dev_private; struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms)); - struct mdp5_state *state = mdp5_get_state(s); + struct mdp5_global_state *state = mdp5_get_global_state(s); struct mdp5_hw_pipe_state *new_state = &state->hwpipe; if (!hwpipe) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c index ae4983d9d0a5..96c2b828dba4 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c @@ -340,17 +340,20 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p) struct mdp5_kms *mdp5_kms = get_kms(smp); struct mdp5_hw_pipe_state *hwpstate; struct mdp5_smp_state *state; + struct mdp5_global_state *global_state; int total = 0, i, j; drm_printf(p, "name\tinuse\tplane\n"); drm_printf(p, "----\t-----\t-----\n"); if (drm_can_sleep()) - drm_modeset_lock(&mdp5_kms->state_lock, NULL); + drm_modeset_lock(&mdp5_kms->glob_state_lock, NULL); + + global_state = mdp5_get_existing_global_state(mdp5_kms); /* grab these *after* we hold the state_lock */ - hwpstate = &mdp5_kms->state->hwpipe; - state = &mdp5_kms->state->smp; + hwpstate = &global_state->hwpipe; + state = &global_state->smp; for (i = 0; i < mdp5_kms->num_hwpipes; i++) { struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i]; @@ -374,7 +377,7 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p) bitmap_weight(state->state, smp->blk_cnt)); if (drm_can_sleep()) - drm_modeset_unlock(&mdp5_kms->state_lock); + drm_modeset_unlock(&mdp5_kms->glob_state_lock); } void mdp5_smp_destroy(struct mdp5_smp *smp) @@ -384,7 +387,8 @@ void mdp5_smp_destroy(struct mdp5_smp *smp) struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms, const struct mdp5_smp_block *cfg) { - struct mdp5_smp_state *state = &mdp5_kms->state->smp; + struct mdp5_smp_state *state; + struct mdp5_global_state *global_state; struct mdp5_smp *smp = NULL; int ret; @@ -398,6 +402,9 @@ struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms, const struct mdp5_smp_ smp->blk_cnt = cfg->mmb_count; smp->blk_size = cfg->mmb_size; + global_state = mdp5_get_existing_global_state(mdp5_kms); + state = &global_state->smp; + /* statically tied MMBs cannot be re-allocated: */ bitmap_copy(state->state, cfg->reserved_state, smp->blk_cnt); memcpy(smp->reserved, cfg->reserved, sizeof(smp->reserved)); -- cgit v1.2.3-59-g8ed1b From c21c731d93e8148d926a63797d33075128e60cdd Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Wed, 21 Feb 2018 09:37:24 -0500 Subject: drm/msm: Don't subclass drm_atomic_state anymore With the addition of "private_objs" in drm_atomic_state, we no longer need to subclass drm_atomic_state to store state of share resources that don't perfectly fit within planes/crtc/connector state information. We can now save this state within drm_atomic_state itself using the private objects. Remove the infrastructure that allowed subclassing of drm_atomic_state in the driver. Changes in v3: - Added to the msm atomic helper patch set Changes in v4: - None Signed-off-by: Archit Taneja Signed-off-by: Rob Clark Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 46 -------------------------------- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 22 --------------- drivers/gpu/drm/msm/msm_atomic.c | 31 --------------------- drivers/gpu/drm/msm/msm_drv.c | 3 --- drivers/gpu/drm/msm/msm_kms.h | 14 ---------- 5 files changed, 116 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 6ada098dba0b..6e12e275deba 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -70,42 +70,6 @@ static int mdp5_hw_init(struct msm_kms *kms) return 0; } -struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s) -{ - struct msm_drm_private *priv = s->dev->dev_private; - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms)); - struct msm_kms_state *state = to_kms_state(s); - struct mdp5_state *new_state; - int ret; - - if (state->state) - return state->state; - - ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx); - if (ret) - return ERR_PTR(ret); - - new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL); - if (!new_state) - return ERR_PTR(-ENOMEM); - - /* Copy state: */ - new_state->hwpipe = mdp5_kms->state->hwpipe; - new_state->hwmixer = mdp5_kms->state->hwmixer; - if (mdp5_kms->smp) - new_state->smp = mdp5_kms->state->smp; - - state->state = new_state; - - return new_state; -} - -static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state) -{ - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); - swap(to_kms_state(state)->state, mdp5_kms->state); -} - /* Global/shared object state funcs */ /* @@ -315,7 +279,6 @@ static const struct mdp_kms_funcs kms_funcs = { .irq = mdp5_irq, .enable_vblank = mdp5_enable_vblank, .disable_vblank = mdp5_disable_vblank, - .swap_state = mdp5_swap_state, .prepare_commit = mdp5_prepare_commit, .complete_commit = mdp5_complete_commit, .wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done, @@ -815,8 +778,6 @@ static void mdp5_destroy(struct platform_device *pdev) drm_atomic_private_obj_fini(&mdp5_kms->glob_state); drm_modeset_lock_fini(&mdp5_kms->glob_state_lock); - - kfree(mdp5_kms->state); } static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt, @@ -969,13 +930,6 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev) mdp5_kms->dev = dev; mdp5_kms->pdev = pdev; - drm_modeset_lock_init(&mdp5_kms->state_lock); - mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL); - if (!mdp5_kms->state) { - ret = -ENOMEM; - goto fail; - } - ret = mdp5_global_obj_init(mdp5_kms); if (ret) goto fail; diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h index 76f0ddfca322..854dfd30e829 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h @@ -28,8 +28,6 @@ #include "mdp5_ctl.h" #include "mdp5_smp.h" -struct mdp5_state; - struct mdp5_kms { struct mdp_kms base; @@ -49,12 +47,6 @@ struct mdp5_kms { struct mdp5_cfg_handler *cfg; uint32_t caps; /* MDP capabilities (MDP_CAP_XXX bits) */ - /** - * Global atomic state. Do not access directly, use mdp5_get_state() - */ - struct mdp5_state *state; - struct drm_modeset_lock state_lock; - /* * Global private object state, Do not access directly, use * mdp5_global_get_state() @@ -88,20 +80,6 @@ struct mdp5_kms { }; #define to_mdp5_kms(x) container_of(x, struct mdp5_kms, base) -/* Global atomic state for tracking resources that are shared across - * multiple kms objects (planes/crtcs/etc). - * - * For atomic updates which require modifying global state, - */ -struct mdp5_state { - struct mdp5_hw_pipe_state hwpipe; - struct mdp5_hw_mixer_state hwmixer; - struct mdp5_smp_state smp; -}; - -struct mdp5_state *__must_check -mdp5_get_state(struct drm_atomic_state *s); - /* Global private object state for tracking resources that are shared across * multiple kms objects (planes/crtcs/etc). */ diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index bf5f8c39f34d..9d0a0ca1f0cb 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -224,11 +224,7 @@ int msm_atomic_commit(struct drm_device *dev, * This is the point of no return - everything below never fails except * when the hw goes bonghits. Which means we can commit the new state on * the software side now. - * - * swap driver private state while still holding state_lock */ - if (to_kms_state(state)->state) - priv->kms->funcs->swap_state(priv->kms, state); /* * Everything below can be run asynchronously without the need to grab @@ -262,30 +258,3 @@ error: drm_atomic_helper_cleanup_planes(dev, state); return ret; } - -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev) -{ - struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL); - - if (!state || drm_atomic_state_init(dev, &state->base) < 0) { - kfree(state); - return NULL; - } - - return &state->base; -} - -void msm_atomic_state_clear(struct drm_atomic_state *s) -{ - struct msm_kms_state *state = to_kms_state(s); - drm_atomic_state_default_clear(&state->base); - kfree(state->state); - state->state = NULL; -} - -void msm_atomic_state_free(struct drm_atomic_state *state) -{ - kfree(to_kms_state(state)->state); - drm_atomic_state_default_release(state); - kfree(state); -} diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 30cd514d8f7c..1c89195da4ff 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -42,9 +42,6 @@ static const struct drm_mode_config_funcs mode_config_funcs = { .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = msm_atomic_commit, - .atomic_state_alloc = msm_atomic_state_alloc, - .atomic_state_clear = msm_atomic_state_clear, - .atomic_state_free = msm_atomic_state_free, }; #ifdef CONFIG_DRM_MSM_REGISTER_LOGGING diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index aaa329dc020e..dfd92947de2c 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -40,8 +40,6 @@ struct msm_kms_funcs { irqreturn_t (*irq)(struct msm_kms *kms); int (*enable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc); void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc); - /* swap global atomic state: */ - void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state *state); /* modeset, bracketing atomic_commit(): */ void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state); void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state); @@ -80,18 +78,6 @@ struct msm_kms { struct msm_gem_address_space *aspace; }; -/** - * Subclass of drm_atomic_state, to allow kms backend to have driver - * private global state. The kms backend can do whatever it wants - * with the ->state ptr. On ->atomic_state_clear() the ->state ptr - * is kfree'd and set back to NULL. - */ -struct msm_kms_state { - struct drm_atomic_state base; - void *state; -}; -#define to_kms_state(x) container_of(x, struct msm_kms_state, base) - static inline void msm_kms_init(struct msm_kms *kms, const struct msm_kms_funcs *funcs) { -- cgit v1.2.3-59-g8ed1b From 347b90b406ff6b6f3c9f666a527eb46e0fcd7aaf Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Wed, 28 Feb 2018 14:18:58 -0500 Subject: drm/msm: Refactor complete_commit() to look more the helpers Factor out the commit_tail() portions of complete_commit() into a separate function to facilitate moving to the atomic helpers in future patches. Changes in v2: - None Changes in v3: - Rebased on Archit's private_obj set Changes in v4: - None Cc: Jeykumar Sankaran Reviewed-by: Archit Taneja Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_atomic.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 9d0a0ca1f0cb..c18f0bee20d4 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -97,18 +97,12 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev, } } -/* The (potentially) asynchronous part of the commit. At this point - * nothing can fail short of armageddon. - */ -static void complete_commit(struct msm_commit *c, bool async) +static void msm_atomic_commit_tail(struct drm_atomic_state *state) { - struct drm_atomic_state *state = c->state; struct drm_device *dev = state->dev; struct msm_drm_private *priv = dev->dev_private; struct msm_kms *kms = priv->kms; - drm_atomic_helper_wait_for_fences(dev, state, false); - kms->funcs->prepare_commit(kms, state); drm_atomic_helper_commit_modeset_disables(dev, state); @@ -135,6 +129,19 @@ static void complete_commit(struct msm_commit *c, bool async) drm_atomic_helper_cleanup_planes(dev, state); kms->funcs->complete_commit(kms, state); +} + +/* The (potentially) asynchronous part of the commit. At this point + * nothing can fail short of armageddon. + */ +static void complete_commit(struct msm_commit *c) +{ + struct drm_atomic_state *state = c->state; + struct drm_device *dev = state->dev; + + drm_atomic_helper_wait_for_fences(dev, state, false); + + msm_atomic_commit_tail(state); drm_atomic_state_put(state); @@ -143,7 +150,7 @@ static void complete_commit(struct msm_commit *c, bool async) static void commit_worker(struct work_struct *work) { - complete_commit(container_of(work, struct msm_commit, work), true); + complete_commit(container_of(work, struct msm_commit, work)); } /** @@ -248,7 +255,7 @@ int msm_atomic_commit(struct drm_device *dev, return 0; } - complete_commit(c, false); + complete_commit(c); return 0; -- cgit v1.2.3-59-g8ed1b From db8f4d5d32334b061d0d9c53ec86480377daeaf0 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Tue, 3 Apr 2018 10:42:23 -0400 Subject: drm/msm: Move implicit sync handling to prepare_fb In preparation for moving to atomic helpers, move the implicit sync fence handling out of atomic commit and into the plane->prepare_fb() hook. While we're at it, de-duplicate the mdp*_prepare_fb functions. Changes in v4: - Added Reported-by: Rob Clark Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 17 +---------------- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 16 +--------------- drivers/gpu/drm/msm/msm_atomic.c | 22 ++++++++++++++++++++++ drivers/gpu/drm/msm/msm_drv.h | 2 ++ 4 files changed, 26 insertions(+), 31 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c index 7a1ad3af08e3..20e956e14c21 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c @@ -98,21 +98,6 @@ static const struct drm_plane_funcs mdp4_plane_funcs = { .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, }; -static int mdp4_plane_prepare_fb(struct drm_plane *plane, - struct drm_plane_state *new_state) -{ - struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane); - struct mdp4_kms *mdp4_kms = get_kms(plane); - struct msm_kms *kms = &mdp4_kms->base.base; - struct drm_framebuffer *fb = new_state->fb; - - if (!fb) - return 0; - - DBG("%s: prepare: FB[%u]", mdp4_plane->name, fb->base.id); - return msm_framebuffer_prepare(fb, kms->aspace); -} - static void mdp4_plane_cleanup_fb(struct drm_plane *plane, struct drm_plane_state *old_state) { @@ -152,7 +137,7 @@ static void mdp4_plane_atomic_update(struct drm_plane *plane, } static const struct drm_plane_helper_funcs mdp4_plane_helper_funcs = { - .prepare_fb = mdp4_plane_prepare_fb, + .prepare_fb = msm_atomic_prepare_fb, .cleanup_fb = mdp4_plane_cleanup_fb, .atomic_check = mdp4_plane_atomic_check, .atomic_update = mdp4_plane_atomic_update, diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c index a9f31da7d45a..e09bc53a0e65 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c @@ -245,20 +245,6 @@ static const struct drm_plane_funcs mdp5_plane_funcs = { .atomic_print_state = mdp5_plane_atomic_print_state, }; -static int mdp5_plane_prepare_fb(struct drm_plane *plane, - struct drm_plane_state *new_state) -{ - struct mdp5_kms *mdp5_kms = get_kms(plane); - struct msm_kms *kms = &mdp5_kms->base.base; - struct drm_framebuffer *fb = new_state->fb; - - if (!new_state->fb) - return 0; - - DBG("%s: prepare: FB[%u]", plane->name, fb->base.id); - return msm_framebuffer_prepare(fb, kms->aspace); -} - static void mdp5_plane_cleanup_fb(struct drm_plane *plane, struct drm_plane_state *old_state) { @@ -543,7 +529,7 @@ static void mdp5_plane_atomic_async_update(struct drm_plane *plane, } static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = { - .prepare_fb = mdp5_plane_prepare_fb, + .prepare_fb = msm_atomic_prepare_fb, .cleanup_fb = mdp5_plane_cleanup_fb, .atomic_check = mdp5_plane_atomic_check, .atomic_update = mdp5_plane_atomic_update, diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index c18f0bee20d4..94f9c3e0e7bf 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -16,6 +16,7 @@ */ #include "msm_drv.h" +#include "msm_gem.h" #include "msm_kms.h" #include "msm_gem.h" #include "msm_fence.h" @@ -97,6 +98,27 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev, } } +int msm_atomic_prepare_fb(struct drm_plane *plane, + struct drm_plane_state *new_state) +{ + struct msm_drm_private *priv = plane->dev->dev_private; + struct msm_kms *kms = priv->kms; + struct drm_gem_object *obj; + struct msm_gem_object *msm_obj; + struct dma_fence *fence; + + if (!new_state->fb) + return 0; + + obj = msm_framebuffer_bo(new_state->fb, 0); + msm_obj = to_msm_bo(obj); + fence = reservation_object_get_excl_rcu(msm_obj->resv); + + drm_atomic_set_fence_for_plane(new_state, fence); + + return msm_framebuffer_prepare(new_state->fb, kms->aspace); +} + static void msm_atomic_commit_tail(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 48ed5b9a8580..98e82230b904 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -160,6 +160,8 @@ struct msm_format { uint32_t pixel_format; }; +int msm_atomic_prepare_fb(struct drm_plane *plane, + struct drm_plane_state *new_state); int msm_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state, bool nonblock); struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev); -- cgit v1.2.3-59-g8ed1b From e765ea77b0fd51152e07aa4e6850b81552b76da3 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Wed, 28 Mar 2018 14:41:23 -0400 Subject: drm/msm: Issue queued events when disabling crtc Ensure that any queued events are issued when disabling the crtc. This avoids timeouts when we come back and wait for dependencies (like the previous frame's flip_done). Changes in v2: - None Changes in v3: - Rebased on Archit's private_obj set Changes in v4: - None Reviewed-by: Archit Taneja Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index 76b96081916f..10271359789e 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -430,6 +430,7 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc, struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state); struct mdp5_kms *mdp5_kms = get_kms(crtc); struct device *dev = &mdp5_kms->pdev->dev; + unsigned long flags; DBG("%s", crtc->name); @@ -445,6 +446,14 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc, mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err); pm_runtime_put_sync(dev); + if (crtc->state->event && !crtc->state->active) { + WARN_ON(mdp5_crtc->event); + spin_lock_irqsave(&mdp5_kms->dev->event_lock, flags); + drm_crtc_send_vblank_event(crtc, crtc->state->event); + crtc->state->event = NULL; + spin_unlock_irqrestore(&mdp5_kms->dev->event_lock, flags); + } + mdp5_crtc->enabled = false; } -- cgit v1.2.3-59-g8ed1b From 70db18dca4e0130acb0600ad51c33176b6162ccc Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Wed, 28 Feb 2018 14:19:01 -0500 Subject: drm/msm: Remove msm_commit/worker, use atomic helper commit Moving further towards switching fully to the the atomic helpers, this patch removes the hand-rolled worker nonblock commit code and uses the atomic helpers commit_work model. Changes in v2: - Remove commit_destroy() - Shuffle order of commit_tail calls to further serialize commits - Use stall in swap_state to avoid abandoned events on disable Changes in v3: - Rebased on Archit's private_obj set Changes in v4: - None Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_atomic.c | 153 +++++++++++---------------------------- drivers/gpu/drm/msm/msm_drv.c | 1 - drivers/gpu/drm/msm/msm_drv.h | 4 - 3 files changed, 42 insertions(+), 116 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 94f9c3e0e7bf..95c7868445dd 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -21,66 +21,6 @@ #include "msm_gem.h" #include "msm_fence.h" -struct msm_commit { - struct drm_device *dev; - struct drm_atomic_state *state; - struct work_struct work; - uint32_t crtc_mask; -}; - -static void commit_worker(struct work_struct *work); - -/* block until specified crtcs are no longer pending update, and - * atomically mark them as pending update - */ -static int start_atomic(struct msm_drm_private *priv, uint32_t crtc_mask) -{ - int ret; - - spin_lock(&priv->pending_crtcs_event.lock); - ret = wait_event_interruptible_locked(priv->pending_crtcs_event, - !(priv->pending_crtcs & crtc_mask)); - if (ret == 0) { - DBG("start: %08x", crtc_mask); - priv->pending_crtcs |= crtc_mask; - } - spin_unlock(&priv->pending_crtcs_event.lock); - - return ret; -} - -/* clear specified crtcs (no longer pending update) - */ -static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask) -{ - spin_lock(&priv->pending_crtcs_event.lock); - DBG("end: %08x", crtc_mask); - priv->pending_crtcs &= ~crtc_mask; - wake_up_all_locked(&priv->pending_crtcs_event); - spin_unlock(&priv->pending_crtcs_event.lock); -} - -static struct msm_commit *commit_init(struct drm_atomic_state *state) -{ - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); - - if (!c) - return NULL; - - c->dev = state->dev; - c->state = state; - - INIT_WORK(&c->work, commit_worker); - - return c; -} - -static void commit_destroy(struct msm_commit *c) -{ - end_atomic(c->dev->dev_private, c->crtc_mask); - kfree(c); -} - static void msm_atomic_wait_for_commit_done(struct drm_device *dev, struct drm_atomic_state *old_state) { @@ -148,31 +88,37 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state) msm_atomic_wait_for_commit_done(dev, state); - drm_atomic_helper_cleanup_planes(dev, state); - kms->funcs->complete_commit(kms, state); + + drm_atomic_helper_wait_for_vblanks(dev, state); + + drm_atomic_helper_commit_hw_done(state); + + drm_atomic_helper_cleanup_planes(dev, state); } /* The (potentially) asynchronous part of the commit. At this point * nothing can fail short of armageddon. */ -static void complete_commit(struct msm_commit *c) +static void commit_tail(struct drm_atomic_state *state) { - struct drm_atomic_state *state = c->state; - struct drm_device *dev = state->dev; + drm_atomic_helper_wait_for_fences(state->dev, state, false); - drm_atomic_helper_wait_for_fences(dev, state, false); + drm_atomic_helper_wait_for_dependencies(state); msm_atomic_commit_tail(state); - drm_atomic_state_put(state); + drm_atomic_helper_commit_cleanup_done(state); - commit_destroy(c); + drm_atomic_state_put(state); } -static void commit_worker(struct work_struct *work) +static void commit_work(struct work_struct *work) { - complete_commit(container_of(work, struct msm_commit, work)); + struct drm_atomic_state *state = container_of(work, + struct drm_atomic_state, + commit_work); + commit_tail(state); } /** @@ -191,17 +137,12 @@ int msm_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state, bool nonblock) { struct msm_drm_private *priv = dev->dev_private; - struct msm_commit *c; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; int i, ret; - ret = drm_atomic_helper_prepare_planes(dev, state); - if (ret) - return ret; - /* * Note that plane->atomic_async_check() should fail if we need * to re-assign hwpipe or anything that touches global atomic @@ -209,45 +150,39 @@ int msm_atomic_commit(struct drm_device *dev, * cases. */ if (state->async_update) { + ret = drm_atomic_helper_prepare_planes(dev, state); + if (ret) + return ret; + drm_atomic_helper_async_commit(dev, state); drm_atomic_helper_cleanup_planes(dev, state); return 0; } - c = commit_init(state); - if (!c) { - ret = -ENOMEM; - goto error; - } + ret = drm_atomic_helper_setup_commit(state, nonblock); + if (ret) + return ret; - /* - * Figure out what crtcs we have: - */ - for_each_new_crtc_in_state(state, crtc, crtc_state, i) - c->crtc_mask |= drm_crtc_mask(crtc); + INIT_WORK(&state->commit_work, commit_work); - /* - * Figure out what fence to wait for: - */ - for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { - if ((new_plane_state->fb != old_plane_state->fb) && new_plane_state->fb) { - struct drm_gem_object *obj = msm_framebuffer_bo(new_plane_state->fb, 0); - struct msm_gem_object *msm_obj = to_msm_bo(obj); - struct dma_fence *fence = reservation_object_get_excl_rcu(msm_obj->resv); - - drm_atomic_set_fence_for_plane(new_plane_state, fence); - } + ret = drm_atomic_helper_prepare_planes(dev, state); + if (ret) + return ret; + + if (!nonblock) { + ret = drm_atomic_helper_wait_for_fences(dev, state, true); + if (ret) + goto error; } /* - * Wait for pending updates on any of the same crtc's and then - * mark our set of crtc's as busy: + * This is the point of no return - everything below never fails except + * when the hw goes bonghits. Which means we can commit the new state on + * the software side now. + * + * swap driver private state while still holding state_lock */ - ret = start_atomic(dev->dev_private, c->crtc_mask); - if (ret) - goto err_free; - - BUG_ON(drm_atomic_helper_swap_state(state, false) < 0); + BUG_ON(drm_atomic_helper_swap_state(state, true) < 0); /* * This is the point of no return - everything below never fails except @@ -272,17 +207,13 @@ int msm_atomic_commit(struct drm_device *dev, */ drm_atomic_state_get(state); - if (nonblock) { - queue_work(priv->atomic_wq, &c->work); - return 0; - } - - complete_commit(c); + if (nonblock) + queue_work(system_unbound_wq, &state->commit_work); + else + commit_tail(state); return 0; -err_free: - kfree(c); error: drm_atomic_helper_cleanup_planes(dev, state); return ret; diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 1c89195da4ff..9cec74c79aa2 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -381,7 +381,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) priv->wq = alloc_ordered_workqueue("msm", 0); priv->atomic_wq = alloc_ordered_workqueue("msm:atomic", 0); - init_waitqueue_head(&priv->pending_crtcs_event); INIT_LIST_HEAD(&priv->inactive_list); INIT_LIST_HEAD(&priv->vblank_ctrl.event_list); diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 98e82230b904..2b2688896295 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -117,10 +117,6 @@ struct msm_drm_private { struct workqueue_struct *wq; struct workqueue_struct *atomic_wq; - /* crtcs pending async atomic updates: */ - uint32_t pending_crtcs; - wait_queue_head_t pending_crtcs_event; - unsigned int num_planes; struct drm_plane *planes[16]; -- cgit v1.2.3-59-g8ed1b From d14659f5de7d2822764d6944ce7d8d7570ebfd9b Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Wed, 28 Feb 2018 14:19:05 -0500 Subject: drm/msm: Switch to atomic_helper_commit() Now that all of the msm-specific goo is tucked safely away we can switch over to using the atomic helper commit directly. \o/ Changes in v2: - None Changes in v3: - Rebased on Archit's private_obj set Changes in v4: - None Cc: Abhinav Kumar Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_atomic.c | 139 +-------------------------------------- drivers/gpu/drm/msm/msm_drv.c | 7 +- drivers/gpu/drm/msm/msm_drv.h | 3 +- 3 files changed, 8 insertions(+), 141 deletions(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 95c7868445dd..f0635c3da7f4 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -18,8 +18,6 @@ #include "msm_drv.h" #include "msm_gem.h" #include "msm_kms.h" -#include "msm_gem.h" -#include "msm_fence.h" static void msm_atomic_wait_for_commit_done(struct drm_device *dev, struct drm_atomic_state *old_state) @@ -59,7 +57,7 @@ int msm_atomic_prepare_fb(struct drm_plane *plane, return msm_framebuffer_prepare(new_state->fb, kms->aspace); } -static void msm_atomic_commit_tail(struct drm_atomic_state *state) +void msm_atomic_commit_tail(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; struct msm_drm_private *priv = dev->dev_private; @@ -73,19 +71,6 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state) drm_atomic_helper_commit_modeset_enables(dev, state); - /* NOTE: _wait_for_vblanks() only waits for vblank on - * enabled CRTCs. So we end up faulting when disabling - * due to (potentially) unref'ing the outgoing fb's - * before the vblank when the disable has latched. - * - * But if it did wait on disabled (or newly disabled) - * CRTCs, that would be racy (ie. we could have missed - * the irq. We need some way to poll for pipe shut - * down. Or just live with occasionally hitting the - * timeout in the CRTC disable path (which really should - * not be critical path) - */ - msm_atomic_wait_for_commit_done(dev, state); kms->funcs->complete_commit(kms, state); @@ -96,125 +81,3 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state) drm_atomic_helper_cleanup_planes(dev, state); } - -/* The (potentially) asynchronous part of the commit. At this point - * nothing can fail short of armageddon. - */ -static void commit_tail(struct drm_atomic_state *state) -{ - drm_atomic_helper_wait_for_fences(state->dev, state, false); - - drm_atomic_helper_wait_for_dependencies(state); - - msm_atomic_commit_tail(state); - - drm_atomic_helper_commit_cleanup_done(state); - - drm_atomic_state_put(state); -} - -static void commit_work(struct work_struct *work) -{ - struct drm_atomic_state *state = container_of(work, - struct drm_atomic_state, - commit_work); - commit_tail(state); -} - -/** - * drm_atomic_helper_commit - commit validated state object - * @dev: DRM device - * @state: the driver state object - * @nonblock: nonblocking commit - * - * This function commits a with drm_atomic_helper_check() pre-validated state - * object. This can still fail when e.g. the framebuffer reservation fails. - * - * RETURNS - * Zero for success or -errno. - */ -int msm_atomic_commit(struct drm_device *dev, - struct drm_atomic_state *state, bool nonblock) -{ - struct msm_drm_private *priv = dev->dev_private; - struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; - struct drm_plane *plane; - struct drm_plane_state *old_plane_state, *new_plane_state; - int i, ret; - - /* - * Note that plane->atomic_async_check() should fail if we need - * to re-assign hwpipe or anything that touches global atomic - * state, so we'll never go down the async update path in those - * cases. - */ - if (state->async_update) { - ret = drm_atomic_helper_prepare_planes(dev, state); - if (ret) - return ret; - - drm_atomic_helper_async_commit(dev, state); - drm_atomic_helper_cleanup_planes(dev, state); - return 0; - } - - ret = drm_atomic_helper_setup_commit(state, nonblock); - if (ret) - return ret; - - INIT_WORK(&state->commit_work, commit_work); - - ret = drm_atomic_helper_prepare_planes(dev, state); - if (ret) - return ret; - - if (!nonblock) { - ret = drm_atomic_helper_wait_for_fences(dev, state, true); - if (ret) - goto error; - } - - /* - * This is the point of no return - everything below never fails except - * when the hw goes bonghits. Which means we can commit the new state on - * the software side now. - * - * swap driver private state while still holding state_lock - */ - BUG_ON(drm_atomic_helper_swap_state(state, true) < 0); - - /* - * This is the point of no return - everything below never fails except - * when the hw goes bonghits. Which means we can commit the new state on - * the software side now. - */ - - /* - * Everything below can be run asynchronously without the need to grab - * any modeset locks at all under one conditions: It must be guaranteed - * that the asynchronous work has either been cancelled (if the driver - * supports it, which at least requires that the framebuffers get - * cleaned up with drm_atomic_helper_cleanup_planes()) or completed - * before the new state gets committed on the software side with - * drm_atomic_helper_swap_state(). - * - * This scheme allows new atomic state updates to be prepared and - * checked in parallel to the asynchronous completion of the previous - * update. Which is important since compositors need to figure out the - * composition of the next frame right after having submitted the - * current layout. - */ - - drm_atomic_state_get(state); - if (nonblock) - queue_work(system_unbound_wq, &state->commit_work); - else - commit_tail(state); - - return 0; - -error: - drm_atomic_helper_cleanup_planes(dev, state); - return ret; -} diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 9cec74c79aa2..021a0b6f9a59 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -41,7 +41,11 @@ static const struct drm_mode_config_funcs mode_config_funcs = { .fb_create = msm_framebuffer_create, .output_poll_changed = drm_fb_helper_output_poll_changed, .atomic_check = drm_atomic_helper_check, - .atomic_commit = msm_atomic_commit, + .atomic_commit = drm_atomic_helper_commit, +}; + +static const struct drm_mode_config_helper_funcs mode_config_helper_funcs = { + .atomic_commit_tail = msm_atomic_commit_tail, }; #ifdef CONFIG_DRM_MSM_REGISTER_LOGGING @@ -438,6 +442,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) } ddev->mode_config.funcs = &mode_config_funcs; + ddev->mode_config.helper_private = &mode_config_helper_funcs; ret = drm_vblank_init(ddev, priv->num_crtcs); if (ret < 0) { diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 2b2688896295..b2da1fbf81e0 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -158,8 +158,7 @@ struct msm_format { int msm_atomic_prepare_fb(struct drm_plane *plane, struct drm_plane_state *new_state); -int msm_atomic_commit(struct drm_device *dev, - struct drm_atomic_state *state, bool nonblock); +void msm_atomic_commit_tail(struct drm_atomic_state *state); struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev); void msm_atomic_state_clear(struct drm_atomic_state *state); void msm_atomic_state_free(struct drm_atomic_state *state); -- cgit v1.2.3-59-g8ed1b From 74d3a3a70775de356b96b5461c3a204a51496fb3 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Thu, 31 May 2018 14:48:58 -0400 Subject: drm/msm: Fix NULL deref on bind/probe deferral This patch avoids dereferencing msm_host->dev when it is NULL. If we find ourselves tearing down dsi before calling (mdp4|mdp5|dpu)_kms_init(), we'll end up in a state where the dev pointer is NULL and trying to extract priv from it will fail. This was introduced in a seemingly innocuous commit to ensure the arguments to msm_gem_put_iova() are correct (even though that function has been a stub for ~5 years). Correctness FTW! \o/ Fixes: b01884a286b0 drm/msm: use correct aspace pointer in msm_gem_put_iova() Cc: Daniel Mack Cc: Rob Clark Signed-off-by: Sean Paul Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/dsi/dsi_host.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers/gpu') diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index b916f464f4ec..2f1a2780658a 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1066,8 +1066,18 @@ static int dsi_tx_buf_alloc(struct msm_dsi_host *msm_host, int size) static void dsi_tx_buf_free(struct msm_dsi_host *msm_host) { struct drm_device *dev = msm_host->dev; - struct msm_drm_private *priv = dev->dev_private; + struct msm_drm_private *priv; + + /* + * This is possible if we're tearing down before we've had a chance to + * fully initialize. A very real possibility if our probe is deferred, + * in which case we'll hit msm_dsi_host_destroy() without having run + * through the dsi_tx_buf_alloc(). + */ + if (!dev) + return; + priv = dev->dev_private; if (msm_host->tx_gem_obj) { msm_gem_put_iova(msm_host->tx_gem_obj, priv->kms->aspace); drm_gem_object_put_unlocked(msm_host->tx_gem_obj); -- cgit v1.2.3-59-g8ed1b