[PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()
Remove obsolete drm_helper_crtc_enable_color_mgmt(). The function is
replaced by drm_crtc_enable_color_mgmt().
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/drm_crtc_helper.c | 33 -
include/drm/drm_crtc_helper.h | 3 ---
2 files changed, 36 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index a6e4243..bf10d70 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1121,36 +1121,3 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc,
int x, int y,
return drm_plane_helper_commit(plane, plane_state, old_fb);
}
EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
-
-/**
- * drm_helper_crtc_enable_color_mgmt - enable color management properties
- * @crtc: DRM CRTC
- * @degamma_lut_size: the size of the degamma lut (before CSC)
- * @gamma_lut_size: the size of the gamma lut (after CSC)
- *
- * This function lets the driver enable the color correction properties on a
- * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
- * set and 2 size properties to inform the userspace of the lut sizes.
- */
-void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
- int degamma_lut_size,
- int gamma_lut_size)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_mode_config *config = &dev->mode_config;
-
- drm_object_attach_property(&crtc->base,
- config->degamma_lut_property, 0);
- drm_object_attach_property(&crtc->base,
- config->ctm_property, 0);
- drm_object_attach_property(&crtc->base,
- config->gamma_lut_property, 0);
-
- drm_object_attach_property(&crtc->base,
- config->degamma_lut_size_property,
- degamma_lut_size);
- drm_object_attach_property(&crtc->base,
- config->gamma_lut_size_property,
- gamma_lut_size);
-}
-EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 97fa894..4b37afa 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -48,9 +48,6 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
int x, int y,
struct drm_framebuffer *old_fb);
-extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
- int degamma_lut_size,
- int gamma_lut_size);
extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
--
1.9.1
[PATCH v4 6/7] drm/i915: Use drm_crtc_enable_color_mgmt()
Use drm_crtc_enable_color_mgmt() in intel_color_init() instead of obsolete drm_helper_crtc_enable_color_mgmt(). Signed-off-by: Jyri Sarha CC: Lionel Landwerlin --- I have not tested this change, only compiled it, but functionally it should be the same. Probably it would make sense to get rid off the condition for calling drm_crtc_enable_color_mgmt() all together. drivers/gpu/drm/i915/intel_color.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 1b3f974..522f5a2 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -547,7 +547,8 @@ void intel_color_init(struct drm_crtc *crtc) /* Enable color management support when we have degamma & gamma LUTs. */ if (INTEL_INFO(dev)->color.degamma_lut_size != 0 && INTEL_INFO(dev)->color.gamma_lut_size != 0) - drm_helper_crtc_enable_color_mgmt(crtc, + drm_crtc_enable_color_mgmt(crtc, INTEL_INFO(dev)->color.degamma_lut_size, + true, INTEL_INFO(dev)->color.gamma_lut_size); } -- 1.9.1
[PATCH v4 5/7] drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties
Use new drm_crtc_enable_color_mgmt() to enable gamma lut properties.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5b7f6f5..e03b349 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -556,16 +556,10 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
* gamma table is not supprted.
*/
if (dispc_mgr_gamma_size(channel)) {
- struct drm_mode_config *config = &dev->mode_config;
- uint gamma_size = 256;
+ uint gamma_lut_size = 256;
- drm_mode_crtc_set_gamma_size(crtc, gamma_size);
-
- drm_object_attach_property(&crtc->base,
- config->gamma_lut_property, 0);
- drm_object_attach_property(&crtc->base,
- config->gamma_lut_size_property,
- gamma_size);
+ drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size);
+ drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size);
}
omap_plane_install_properties(crtc->primary, &crtc->base);
--
1.9.1
[PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt()
Add drm_crtc_enable_color_mgmt() to. The new function makes the old
drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more
flexible. It allows driver to enable only the feature it has without
forcing to enable all three color management features: gegamma lut, csc
matrix (ctm), and gamma lut.
Suggested-by: Daniel Vetter
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/drm_crtc.c | 45 +
include/drm/drm_crtc.h | 5 -
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d2a6d95..b097226 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -6065,3 +6065,48 @@ struct drm_tile_group *drm_mode_create_tile_group(struct
drm_device *dev,
return tg;
}
EXPORT_SYMBOL(drm_mode_create_tile_group);
+
+/**
+ * drm_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @has_ctm: whether to attach ctm_property for CSC matrix
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction
+ * properties on a CRTC. This includes 3 degamma, csc and gamma
+ * properties that userspace can set and 2 size properties to inform
+ * the userspace of the lut sizes. Each of the properties are
+ * optional. The gamma and degamma properties are only attached if
+ * their size is not 0 and ctm_property is only attached if has_ctm is
+ * true.
+ */
+void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+ uint degamma_lut_size,
+ bool has_ctm,
+ uint gamma_lut_size)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+
+ if (degamma_lut_size) {
+ drm_object_attach_property(&crtc->base,
+ config->degamma_lut_property, 0);
+ drm_object_attach_property(&crtc->base,
+ config->degamma_lut_size_property,
+ degamma_lut_size);
+ }
+
+ if (has_ctm)
+ drm_object_attach_property(&crtc->base,
+ config->ctm_property, 0);
+
+ if (gamma_lut_size) {
+ drm_object_attach_property(&crtc->base,
+ config->gamma_lut_property, 0);
+ drm_object_attach_property(&crtc->base,
+ config->gamma_lut_size_property,
+ gamma_lut_size);
+ }
+}
+EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d1559cd..36d3bbf 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2553,7 +2553,10 @@ extern struct drm_property
*drm_mode_create_rotation_property(struct drm_device
unsigned int
supported_rotations);
extern unsigned int drm_rotation_simplify(unsigned int rotation,
unsigned int supported_rotations);
-
+extern void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+ uint degamma_lut_size,
+ bool has_ctm,
+ uint gamma_lut_size);
/* Helpers */
static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
--
1.9.1
[PATCH v4 3/7] drm/omapdrm: Implement gamma_lut atomic crtc properties
Implement gamma_lut atomic crtc properties, set crtc gamma size to 256
for all crtcs and use drm_atomic_helper_legacy_gamma_set() as
gamma_set func. The tv-out crtc has 1024 element gamma table (with
10bit precision) in HW, but current Xorg server does not accept
anything else but 256 elements so that is used for all CRTCs. The dss
dispc API converts table of any length for HW and uses linear
interpolation in the process. The default gamma table is restored
gamma_lut property is deleted.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 075f2bb..5b7f6f5 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -384,6 +384,19 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
WARN_ON(omap_crtc->vblank_irq.registered);
+ if (crtc->state->color_mgmt_changed) {
+ struct drm_color_lut *lut = NULL;
+ uint length = 0;
+
+ if (crtc->state->gamma_lut) {
+ lut = (struct drm_color_lut *)
+ crtc->state->gamma_lut->data;
+ length = crtc->state->gamma_lut->length /
+ sizeof(*lut);
+ }
+ dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
+ }
+
if (dispc_mgr_is_enabled(omap_crtc->channel)) {
DBG("%s: GO", omap_crtc->name);
@@ -460,6 +473,7 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
.set_config = drm_atomic_helper_set_config,
.destroy = omap_crtc_destroy,
.page_flip = drm_atomic_helper_page_flip,
+ .gamma_set = drm_atomic_helper_legacy_gamma_set,
.set_property = drm_atomic_helper_crtc_set_property,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
@@ -534,6 +548,26 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
drm_crtc_helper_add(crtc, &omap_crtc_helper_funcs);
+ /* The dispc API adapts to what ever size, but the HW supports
+* 256 element gamma table for LCDs and 1024 element table for
+* OMAP_DSS_CHANNEL_DIGIT. X server assumes 256 element gamma
+* tables so lets use that. Size of HW gamma table can be
+* extracted with dispc_mgr_gamma_size(). If it returns 0
+* gamma table is not supprted.
+*/
+ if (dispc_mgr_gamma_size(channel)) {
+ struct drm_mode_config *config = &dev->mode_config;
+ uint gamma_size = 256;
+
+ drm_mode_crtc_set_gamma_size(crtc, gamma_size);
+
+ drm_object_attach_property(&crtc->base,
+ config->gamma_lut_property, 0);
+ drm_object_attach_property(&crtc->base,
+ config->gamma_lut_size_property,
+ gamma_size);
+ }
+
omap_plane_install_properties(crtc->primary, &crtc->base);
omap_crtcs[channel] = omap_crtc;
--
1.9.1
[PATCH v4 2/7] drm/omapdrm: Workaround for errata i734 (LCD1 Gamma) in DSS dispc
Workaround for errata i734 in DSS dispc
- LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled
For gamma tables to work on LCD1 the GFX plane has to be used at least
once after DSS HW has come out of reset. The workaround sets up a
minimal LCD setup with GFX plane and waits for one vertical sync irq
before disabling the setup and continuing with the context
restore. The physical outputs are gated during the operation.
For details see:
OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata
Literature Number: SWPZ037E
Or some other relevant errata document for the DSS IP version.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/omapdrm/dss/dispc.c | 174
1 file changed, 174 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index dbe5ab3..eff857f 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -115,6 +115,8 @@ struct dispc_features {
bool reverse_ilace_field_order:1;
bool has_gamma_table:1;
+
+ bool has_gamma_i734_bug:1;
};
#define DISPC_MAX_NR_FIFOS 5
@@ -4095,6 +4097,7 @@ static const struct dispc_features omap44xx_dispc_feats =
{
.supports_double_pixel = true,
.reverse_ilace_field_order =true,
.has_gamma_table= true,
+ .has_gamma_i734_bug = true,
};
static const struct dispc_features omap54xx_dispc_feats = {
@@ -4121,6 +4124,7 @@ static const struct dispc_features omap54xx_dispc_feats =
{
.supports_double_pixel = true,
.reverse_ilace_field_order =true,
.has_gamma_table= true,
+ .has_gamma_i734_bug = true,
};
static int dispc_init_features(struct platform_device *pdev)
@@ -4212,6 +4216,168 @@ void dispc_free_irq(void *dev_id)
}
EXPORT_SYMBOL(dispc_free_irq);
+/*
+ * Workaround for errata i734 in DSS dispc
+ * - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled
+ *
+ * For gamma tables to work on LCD1 the GFX plane has to be used at
+ * least once after DSS HW has come out of reset. The workaround
+ * sets up a minimal LCD setup with GFX plane and waits for one
+ * vertical sync irq before disabling the setup and continuing with
+ * the context restore. The physical outputs are gated during the
+ * operation. This workaround requires that gamma table's LOADMODE
+ * is set to 0x2 in DISPC_CONTROL1 register.
+ *
+ * For details see:
+ * OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata
+ * Literature Number: SWPZ037E
+ * Or some other relevant errata document for the DSS IP version.
+ */
+
+static const struct dispc_errata_i734_data {
+ struct omap_video_timings timings;
+ struct omap_overlay_info ovli;
+ struct omap_overlay_manager_info mgri;
+ struct dss_lcd_mgr_config lcd_conf;
+} i734 = {
+ .timings = {
+ .x_res = 8, .y_res = 1,
+ .pixelclock = 1600,
+ .hsw = 8, .hfp = 4, .hbp = 4,
+ .vsw = 1, .vfp = 1, .vbp = 1,
+ .vsync_level = OMAPDSS_SIG_ACTIVE_LOW,
+ .hsync_level = OMAPDSS_SIG_ACTIVE_LOW,
+ .interlace = false,
+ .data_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE,
+ .de_level = OMAPDSS_SIG_ACTIVE_HIGH,
+ .sync_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE,
+ .double_pixel = false,
+ },
+ .ovli = {
+ .screen_width = 1,
+ .width = 1, .height = 1,
+ .color_mode = OMAP_DSS_COLOR_RGB24U,
+ .rotation = OMAP_DSS_ROT_0,
+ .rotation_type = OMAP_DSS_ROT_DMA,
+ .mirror = 0,
+ .pos_x = 0, .pos_y = 0,
+ .out_width = 0, .out_height = 0,
+ .global_alpha = 0xff,
+ .pre_mult_alpha = 0,
+ .zorder = 0,
+ },
+ .mgri = {
+ .default_color = 0,
+ .trans_enabled = false,
+ .partial_alpha_enabled = false,
+ .cpr_enable = false,
+ },
+ .lcd_conf = {
+ .io_pad_mode = DSS_IO_PAD_MODE_BYPASS,
+ .stallmode = false,
+ .fifohandcheck = false,
+ .clock_info = {
+ .lck_div = 1,
+ .pck_div = 2,
+ },
+ .video_port_width = 24,
+ .lcden_sig_polarity = 0,
+ },
+};
+
+static struct i734_buf {
+ size_t size;
+ dma_addr_t paddr;
+ void *vaddr;
+} i734_buf;
+
+static int dispc_errata_i734_wa_init(void)
+{
+ if (!dispc.feat->has_gamma_i734_bug)
+ return 0;
+
+ i734_buf.size = i734.ovli.width * i734.ovli.height *
+ color_mode_to_bpp(i734.ovli.color_mode) / 8;
+
+ i734_buf.vaddr = dma_alloc_writecombine(&dispc.pdev->dev, i734_buf.size,
+
[PATCH v4 1/7] drm/omapdrm: Add gamma table support to DSS dispc
Add gamma table support to DSS dispc.
DSS driver initializes the default gamma table at component bind time
and holds a copy of all gamma tables in its internal data structure.
Each call to dispc_mgr_set_gamma() updates the internal table and
triggers write to the HW, if it is enabled. The tables are restored to
HW in PM resume callback. The drivers internal data structure match
the HW tables in size and in number of significant bits per color
component. The dispc_mgr_set_gamma() converts the size of any given
table for the internal data structure using linear interpolation.
Default gamma table is restored if NULL is given in place of gamma
lut.
dispc_mgr_gamma_size() gives HW gamma table size for the channel and
returns 0 if gamma table is not supported by the HW or the DSS driver.
Signed-off-by: Jyri Sarha
---
drivers/gpu/drm/omapdrm/dss/dispc.c | 200 +++---
drivers/gpu/drm/omapdrm/dss/dispc.h | 5 +
drivers/gpu/drm/omapdrm/dss/hdmi4.c | 3 -
drivers/gpu/drm/omapdrm/dss/hdmi5.c | 3 -
drivers/gpu/drm/omapdrm/dss/omapdss.h | 5 +
5 files changed, 194 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index f83608b..dbe5ab3 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -113,9 +113,12 @@ struct dispc_features {
* never both, we can just use this flag for now.
*/
bool reverse_ilace_field_order:1;
+
+ bool has_gamma_table:1;
};
#define DISPC_MAX_NR_FIFOS 5
+#define DISPC_MAX_CHANNEL_GAMMA 4
static struct {
struct platform_device *pdev;
@@ -135,6 +138,8 @@ static struct {
boolctx_valid;
u32 ctx[DISPC_SZ_REGS / sizeof(u32)];
+ u32 *gamma_table[DISPC_MAX_CHANNEL_GAMMA];
+
const struct dispc_features *feat;
bool is_enabled;
@@ -178,11 +183,19 @@ struct dispc_reg_field {
u8 low;
};
+struct dispc_gamma_desc {
+ u32 len;
+ u32 bits;
+ u16 reg;
+ bool has_index;
+};
+
static const struct {
const char *name;
u32 vsync_irq;
u32 framedone_irq;
u32 sync_lost_irq;
+ struct dispc_gamma_desc gamma;
struct dispc_reg_field reg_desc[DISPC_MGR_FLD_NUM];
} mgr_desc[] = {
[OMAP_DSS_CHANNEL_LCD] = {
@@ -190,6 +203,12 @@ static const struct {
.vsync_irq = DISPC_IRQ_VSYNC,
.framedone_irq = DISPC_IRQ_FRAMEDONE,
.sync_lost_irq = DISPC_IRQ_SYNC_LOST,
+ .gamma = {
+ .len= 256,
+ .bits = 8,
+ .reg= DISPC_GAMMA_TABLE0,
+ .has_index = true,
+ },
.reg_desc = {
[DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL, 0,
0 },
[DISPC_MGR_FLD_STNTFT] = { DISPC_CONTROL, 3,
3 },
@@ -207,6 +226,12 @@ static const struct {
.vsync_irq = DISPC_IRQ_EVSYNC_ODD | DISPC_IRQ_EVSYNC_EVEN,
.framedone_irq = DISPC_IRQ_FRAMEDONETV,
.sync_lost_irq = DISPC_IRQ_SYNC_LOST_DIGIT,
+ .gamma = {
+ .len= 1024,
+ .bits = 10,
+ .reg= DISPC_GAMMA_TABLE2,
+ .has_index = false,
+ },
.reg_desc = {
[DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL, 1,
1 },
[DISPC_MGR_FLD_STNTFT] = { },
@@ -224,6 +249,12 @@ static const struct {
.vsync_irq = DISPC_IRQ_VSYNC2,
.framedone_irq = DISPC_IRQ_FRAMEDONE2,
.sync_lost_irq = DISPC_IRQ_SYNC_LOST2,
+ .gamma = {
+ .len= 256,
+ .bits = 8,
+ .reg= DISPC_GAMMA_TABLE1,
+ .has_index = true,
+ },
.reg_desc = {
[DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL2, 0,
0 },
[DISPC_MGR_FLD_STNTFT] = { DISPC_CONTROL2, 3,
3 },
@@ -241,6 +272,12 @@ static const struct {
.vsync_irq = DISPC_IRQ_VSYNC3,
.framedone_irq = DISPC_IRQ_FRAMEDONE3,
.sync_lost_irq = DISPC_IRQ_SYNC_LOST3,
+ .gamma = {
+ .len= 256,
+ .bits = 8,
+ .reg= DISPC_GAMMA_TABLE3,
+ .has_index = true,
+ },
.reg_desc = {
[DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL3, 0,
0 },
[DISPC_MGR_FLD_STNTFT] = { DISPC_CONTROL3, 3,
3 },
@@ -1084,20 +1121,6 @@ static u32 d
[PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt()
Implements gamma tables for OMAP4, OMAP5, and dra7xx SoCs and adds a
work-a-round for errata that may break LCD1 channel if gamma tables
are in use.
Also adds new drm_crtc_enable_color_mgmt() as suggested[1] by Daniel
Vetter and get rid of the old drm_helper_crtc_enable_color_mgmt(). I
have not tested the change to i915 driver, only compiled it, but
functionally it should be exactly the same.
[1] http://www.spinics.net/lists/dri-devel/msg108092.html
Changes from v3 to v4
- "drm/omapdrm: Add gamma table support to DSS dispc"
- use interpolation code in dispc_mgr_set_gamma() to produce default lut
- restore default lut table if NULL is given as gamma lut table
- "drm/omapdrm: Implement gamma_lut atomic crtc property"
- attach gamma_lut_property and gamma_lut_size_property to crtc if
gamma tables are supported
- restore default table if gamma lut property is deleted
- Adds:
- drm: Add drm_crtc_enable_color_mgmt()
- drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties
- drm/i915: Use drm_crtc_enable_color_mgmt()
- drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()
Changes from v2 to v3
- "drm/omapdrm: Add gamma table support to DSS dispc"
- fix dispc_init_gamma_tables() to always return an int
- omap54xx_dispc_feats initializes .has_gamma_table not .has_gamma_tables
- "drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc"
- work-a-round -> workaround
- Do not mention LOADMODE in description
- dma_alloc_writecombine returns NULL on error
- fix last wrong instrance of LCD output gating register
- improve comment for framedone busy wait
- add {} around busy loop's while statement
Changes from v1 to v2
- Drop "drm/omapdrm: omap_modeset_init: Separate crtc id and plane id indexing"
- "drm/omapdrm: Add gamma table support to DSS dispc"
- Address Tomi's comments here: https://patchwork.kernel.org/patch/9128629/
- "drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc"
- Address Tomi's comments here: https://patchwork.kernel.org/patch/9128633/
Jyri Sarha (7):
drm/omapdrm: Add gamma table support to DSS dispc
drm/omapdrm: Workaround for errata i734 (LCD1 Gamma) in DSS dispc
drm/omapdrm: Implement gamma_lut atomic crtc properties
drm: Add drm_crtc_enable_color_mgmt()
drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma
properties
drm/i915: Use drm_crtc_enable_color_mgmt()
drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()
drivers/gpu/drm/drm_crtc.c| 45
drivers/gpu/drm/drm_crtc_helper.c | 33 ---
drivers/gpu/drm/i915/intel_color.c| 3 +-
drivers/gpu/drm/omapdrm/dss/dispc.c | 374 --
drivers/gpu/drm/omapdrm/dss/dispc.h | 5 +
drivers/gpu/drm/omapdrm/dss/hdmi4.c | 3 -
drivers/gpu/drm/omapdrm/dss/hdmi5.c | 3 -
drivers/gpu/drm/omapdrm/dss/omapdss.h | 5 +
drivers/gpu/drm/omapdrm/omap_crtc.c | 28 +++
include/drm/drm_crtc.h| 5 +-
include/drm/drm_crtc_helper.h | 3 -
11 files changed, 447 insertions(+), 60 deletions(-)
--
1.9.1
[Bug 95517] Constant GPU VM faults
https://bugs.freedesktop.org/show_bug.cgi?id=95517 --- Comment #4 from lumetili at gmail.com --- OK my 3D accelerated VirtualBox VM just got stuck and journalctl -kf started spitting out VM faults once a second in a loop, at first there's a: May 26 01:39:24 carrier kernel: radeon :02:00.0: GPU fault detected: 146 0x0042080c May 26 01:39:24 carrier kernel: radeon :02:00.0: VM_CONTEXT1_PROTECTION_FAULT_ADDR 0x00014482 May 26 01:39:24 carrier kernel: radeon :02:00.0: VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x0200800C May 26 01:39:24 carrier kernel: VM fault (0x0c, vmid 1) at page 83074, read from TC (8) And then a: May 26 01:39:24 carrier kernel: radeon :02:00.0: GPU fault detected: 146 0x0390350c May 26 01:39:24 carrier kernel: radeon :02:00.0: VM_CONTEXT1_PROTECTION_FAULT_ADDR 0xC49C May 26 01:39:24 carrier kernel: radeon :02:00.0: VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x1003500C May 26 01:39:24 carrier kernel: VM fault (0x0c, vmid 8) at page 50332, read from VGT (53) It devolves into looping the VGT entry with VM_CONTEXT1_PROTECTION_FAULT_ADDR 0xC4A with NUMBER cycling from 0..F -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/2f2954bb/attachment-0001.html>
[PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt()
On 25 May 2016 at 22:05, Emil Velikov wrote: > On 25 May 2016 at 21:43, Jyri Sarha wrote: >> Add drm_crtc_enable_color_mgmt() to. The new function makes the old >> drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more >> flexible. It allows driver to enable only the feature it has without >> forcing to enable all three color management features: gegamma lut, csc >> matrix (ctm), and gamma lut. >> > Why don't we just update the existing one ? In one step (patch) or two > - a) don't register property if respective _lut_size is zero b) bring > in has_ctm. > Ouch I missed that the goal here is to move the function out of drm_crtc_helper.c. Sorry about that. Perhaps the commit message can be reworded a bit - something like the following comes to mind. "Introduce drm_crtc_enable_color_mgmt() function which supersedes the crtc_helper one (xxx: add name). The latter always creates all properties (xxx: list props) and is not really a helper. The new version allows explicit control of the properties created as required by some hardware/drivers (xxx: name driver)." Feel free to reuse and/tweak. Thanks Emil
[PATCH] drm/amd: add Kconfig dependency for ACP on DRM_AMDGPU
Hi Jeff, I'm thinking out loud so here are a few suggestions/ideas. Please don't take them too seriously although they do make sense from this end. On 24 May 2016 at 18:47, Jeff Mahoney wrote: > The DRM_AMD_ACP option doesn't have any dependencies and selects > MFD_CORE, which results in MFD_CORE=y. Since the code is only called > from DRM_AMDGPU, it should depend on it. Adding the dependency results > in MFD_CORE being selected as a module again if amdgpu is also a module. > > Signed-off-by: Jeff Mahoney > --- > drivers/gpu/drm/amd/acp/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/acp/Kconfig b/drivers/gpu/drm/amd/acp/Kconfig > index ca77ec1..e503e3d 100644 > --- a/drivers/gpu/drm/amd/acp/Kconfig > +++ b/drivers/gpu/drm/amd/acp/Kconfig > @@ -2,6 +2,7 @@ menu "ACP (Audio CoProcessor) Configuration" > > config DRM_AMD_ACP > bool "Enable AMD Audio CoProcessor IP support" > + depends on DRM_AMDGPU > select MFD_CORE > select PM_GENERIC_DOMAINS if PM > help > Afaict ACP/Powerplay doesn't make sense on their own so here is an alternative solution: - create a Kconfig in drm/amd and move/consolidate amdgpu, powerplay, acp in there. amdkfd can be moved as well afaict. - make DRM_AMD_ACP and DRM_AMD_POWERPLAY submenu items for DRM_AMDGPU. AMDKFD on the other hand depends on RADEON || AMDGPU so it should stay separate. - crazy idea: add select and/or depends on SND_SOC_AMD_ACP. since one is useless without the other. Or perhaps make the SOC entry should select/depend on AMD_ACP ? And some future work (cleanups): - fold the acp folder (or inline respective files) within amdgpu. - add forward declarations in acp/include/acp_gfx_if.h and move the cgs* includes where needed (acp/acp_hw.c and amdgpu/amdgpu_acp.c) - apply similar polish for amdgpu/amdgpu_acp.h. - drop the (unneeded ?) include of amdgpu_acp.h include from amdgpu/vi.c - kill off amdgpu_acp::private. Afaict there's not a single place (as of 95306975e9dd38ba2775dd96cb29987ecc7d9360) that actually defines the amd_acp_private struct. Is something broken on my end or this does not build without warnings/errors ? - kill off amdgpu_acp::acp_genpd::cgs_dev (use amdgpu_acp::cgs_device directly) and inline the ::acp_genpd::gpd into amdgpu_acp. Regards, Emil
[PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt()
On 25 May 2016 at 21:43, Jyri Sarha wrote: > Add drm_crtc_enable_color_mgmt() to. The new function makes the old > drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more > flexible. It allows driver to enable only the feature it has without > forcing to enable all three color management features: gegamma lut, csc > matrix (ctm), and gamma lut. > Why don't we just update the existing one ? In one step (patch) or two - a) don't register property if respective _lut_size is zero b) bring in has_ctm. Regards, Emil
[Bug 118701] x86_64 GMA500 reports /dev/dri/card0: failed to set DRM interface version 1.4: Inappropriate ioctl for device
https://bugzilla.kernel.org/show_bug.cgi?id=118701 --- Comment #5 from Patrik Jakobsson --- Thanks for testing, queued for next and stable -- You are receiving this mail because: You are watching the assignee of the bug.
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 7:57 PM, Rob Clark wrote: >> As far as virtualizing the resources goes. I think that would need a >> whole new API. Or at least a separate set of objects perhaps. I'm too >> lazy to dig up all the old arguments now, but I'm pretty sure there >> were many. If this would be done, I suspect the only sane way to do it >> would be to just have a hwc implementation in the kernel. As in user >> space would pass in the desired configuration, and the kernel would >> assign resources as best it can and return the result back to userspace. > > actually, without introducing a completely new/different uapi, it > should work pretty reasonably with the way weston atomic works, > incrementally adding planes in TESTONLY steps until it fails. I > suspect on some more creative hw (msm is prime candidate) we'll still > see custom hwc implementations to squeeze out the last bit of > performance/power. But with the approach that weston uses a generic > userspace should still work pretty well on that hw. So summary from my points on the m-l: - Whether we do this in all drivers that need this for dual-pipe 4k, or in all compositors is probably similar amounts of work. The upside of doing it in drivers is that it's closer to the metal, avoid midlayer-like mistakes in trying to funnel tons of stuff between the top and bottom layers through drm core, and still failing in corner cases. Drivers can simply fix things up if they're in control. - For a bunch of reasons we (well Ville is really big on those) want to add clipped_src/dst rectangles to drm_plane_state. That's a bit of churn, but it's per-driver opt-in churn, so manageable. Once we have clipped rectangles and so fully decoupled the hw state from the uabi state it's easy to clip a plane to a sub-pipe instead of the entire virtual area of the crtc. - Once we can clip planes we can add a bit of helper magic to steal additional&unused planes for use as the 2nd/3rd/4th plane scanning out one logical surface (drm_buffer) attached to a logical plane (drm_plane). Add a helper on top which splits a 4/5k screen at a passed in limit, and this boils down to a few lines in drivers. While still giving the drivers full control over all the details, if they want to. - Really assigning resources within the driver while exposing whether there's enough to userspace using TEST_ONLY is the point of atomic. Doesn't matter whether it's fifo space, scalers, wizzbang gizmo A, B or C or this thing called "scanout engine". If your hw needs more than one of those to get a basic job done (fullscreen primary plane) then it's imo the drivers job to juggle all the internal resources around. KMS is supposed to work from workstation gpus down to soc in a somewhat uniform way, even on crappy hw. Summary: Still firmly in the "virtualize it!" camp ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #20 from Wilfried Klaebe --- No Xorg config. Especially nothing special for intel or radeon. No special rEFInd features. Files attached. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #19 from Wilfried Klaebe --- Created attachment 217581 --> https://bugzilla.kernel.org/attachment.cgi?id=217581&action=edit refind_linux.conf of both cases -- You are receiving this mail because: You are the assignee for the bug.
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #18 from Wilfried Klaebe --- Created attachment 217571 --> https://bugzilla.kernel.org/attachment.cgi?id=217571&action=edit Xorg.log on working Linux 4.6.0 (commit reverted) -- You are receiving this mail because: You are the assignee for the bug.
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #17 from Wilfried Klaebe --- Created attachment 217561 --> https://bugzilla.kernel.org/attachment.cgi?id=217561&action=edit Xorg.log on non-working Linux 4.6.0 -- You are receiving this mail because: You are the assignee for the bug.
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #16 from Wilfried Klaebe --- Created attachment 217551 --> https://bugzilla.kernel.org/attachment.cgi?id=217551&action=edit kern.log / dmesg of working Linux 4.6.0 (commit reverted) -- You are receiving this mail because: You are the assignee for the bug.
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #15 from Wilfried Klaebe --- Created attachment 217541 --> https://bugzilla.kernel.org/attachment.cgi?id=217541&action=edit kern.log / dmesg of non-working Linux 4.6.0 -- You are receiving this mail because: You are the assignee for the bug.
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #14 from Wilfried Klaebe --- Created attachment 217531 --> https://bugzilla.kernel.org/attachment.cgi?id=217531&action=edit config of working Linux 4.6.0 (with commit reverted) -- You are receiving this mail because: You are the assignee for the bug.
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #13 from Wilfried Klaebe --- Created attachment 217521 --> https://bugzilla.kernel.org/attachment.cgi?id=217521&action=edit config of non-working Linux 4.6.0 -- You are receiving this mail because: You are the assignee for the bug.
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 12:36:53PM -0400, Rob Clark wrote: > On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: > > On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: > >> On 05/25/2016 12:40 PM, Daniel Vetter wrote: > >> >- Is the size/width really independent of e.g. rotation/pixel format/... > >> > Should it be the maximum that's possible under best circumstance > >> > (things > >> > could still fail), or the minimum that's guaranteed to work everwhere. > >> > This is the kind of stuff we need the userspace part for, too. > >> > >> Yeah, it isn't independent of these parameters. I'm not entirely sure > >> about this either. > >> > >> Does it make sense to impose a rule that the user first sets the > >> rotation/format plane properties, and only then read the maximum > >> width? I'm assuming user space hates such kind of stuff. > >> > >> If we use the 'best circumstance' max_width, we can first start > >> with a minimum number of planes that need to be grouped to achieve > >> the target mode. If that fails the atomic test, then we can try to > >> add one plane at a time, and reduce the width for each plane. > >> > >> If we use the minimum/'guaranteed to work' max_width, we'll get > >> a higher number of planes than needed for this mode. This would pass > >> the atomic test. We could then reduce a plane at a time and see when > >> we fail the atomic test. > >> > >> I guess we need to chose the one that's more probable to get right > >> the first time. Considering only pixel formats for now, the > >> minimum/'guaranteed to work' would map to the RGB formats. The best > >> circumstance ones would probably be the planar video formats. Since > >> we use RGB more often, the minimum one might make more sense. > >> > >> We could, of course, give the property a range of max widths to > >> confuse user space even more. > > > > An entirely different idea for cases where a simple hint property doesn't > > work (other ideas floating around are can_scale, to give a hint whether a > > plan can at least in theory up/downscale, or not at all), is that the > > kernel gives more specific hints about what it would like to change. > > > > So if userspace asks for a plane, but for the given pixel format it's too > > wide, the kernel could return a new proposed value for width. That would > > be super-flexible and could cover all kinds of use-case like rotation > > needing a specific tiling (fb_modifier) or specific pixel format, or > > specific stride. > > > > For the case at hand there's even more worms: What about stride > > requirements? Afaik on some hw you just need to split the buffers into 2 > > planes, but can keep the wide stride (since the limit is the size of the > > linebuffers in the hw). On others you need to split the buffer itself into > > 2, because the plane hw can't cope with huge strides. Again might depend > > upon the pixel format. > > > > So in a way height/width is both too much information and not precise > > enough. Entirely different approches: > > - We just add a might_need_split_plane prop to crtcs where this might be > > needed. Userspace then gets to retry with split buffers if it doesn't > > work with a huge one. > > > > - When I discussed this with qualcom folks for msm we concluded that the > > simplest approach would be to hide this in the kernel. So if you have a > > too wide plane, and need 2 hw planes to scan it out, then do that > > transparently in the kernel. Of course this means that there will be 1 > > (or 3 if you need a 2x2 split) fewer planes available, but userspace > > needs to iteratively build up the plane config using ATOMIC_TEST anyway. > > Just fwiw, there are a few things that we will still end up > abstracting in the kernel by virtualizing the mapping between kms > planes and hw pipes. And the approach of weston atomic of > incrementally adding more planes w/ TESTONLY flag should work well for > that. (Let's hope the weston bits get upstream some day..) > > But exposing width limit avoids the one-plane to multiple-pipes case, > considerably simplifying things. And seemed like a generic enough > limit (iirc, it applies to omapdss and probably others), that it would > be cleaner to expose the limit to userspace. So there should be at > least a couple other drivers that could avoid virtualizing planes with > some help from userspace for this case. > > Regarding rotation, I'm not 100% sure.. seems like we could just > document these as the un-rotated limits. If we really had to, we > could do some sort of dance where userspace sets rotation property on > an un-used plane, and then reads-back the current values of the > read-only prop's. But that seems awkward. I'm thinking all of this is doomed to fail. So right now people seem to want some kind of maximum size of the source viewport. What about the destination size? Is the max size for unscaled/scaled/smething else? Rotation/pixel format were already mentioned. How does this int
[PATCH] drm: expand cea861 mode timing table
Hi Thierry Reding,
enum hdmi_picture_aspect {
> HDMI_PICTURE_ASPECT_NONE,
> HDMI_PICTURE_ASPECT_4_3,
> HDMI_PICTURE_ASPECT_16_9,
> + HDMI_PICTURE_ASPECT_64_27,
> + HDMI_PICTURE_ASPECT_256_135,
> HDMI_PICTURE_ASPECT_RESERVED,
> };
These are defined since CEA861F defines them in section 4.1.
However, it is not indicated in AVI InfoFrame definition for picture aspect
ratio (M1,M0), and we should indicate (M1,M0) = (0,0) for "No Data" when
sending VICs corresponding to these new aspect ratios.
If (M1,M0) = (0,0) "No Data" is indicated, then If M=0 (M1=0, M0=0) and VIC=0,
a Sink shall assume the Picture is formatted according to the Preferred Picture
Aspect Ratio.
The defition:
Preferred Picture Aspect Ratio-In a Dual-Aspect Ratio DTV, the preferred aspect
ratio of a given Video Format Timing (e.g., 720x480p) is the aspect ratio of
the first such timing listed in the EDID data structure (see Section 4.1). This
would be the Picture Aspect Ratio that would be displayed if a DTV were to
receive a Video Format Timing with no accompanying Picture Aspect Ratio
information (i.e., no AVI sent from Source).
Alternatively, since our code does not actively use HDMI_PICTURE_ASPECT_64_27,
HDMI_PICTURE_ASPECT_256_135, we can unify them as HDMI_PICTURE_ASPECT_NONE, and
send (M1,M0) = (0,0) to avoid confusion.
-Original Message-
From: Thierry Reding
Sent: Friday, May 13, 2016 11:28:39 AM
To: Yang, Eric
Cc: dri-devel at lists.freedesktop.org; linux-fbdev at vger.kernel.org;
tomi.valkeinen at ti.com; plagnioj at jcrosoft.com
Subject: Re: [PATCH] drm: expand cea861 mode timing table
On Thu, May 12, 2016 at 03:37:33PM -0400, Eric Yang wrote:
[...]
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index
> e974420..edbb4fc 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -78,6 +78,8 @@ enum hdmi_picture_aspect {
> HDMI_PICTURE_ASPECT_NONE,
> HDMI_PICTURE_ASPECT_4_3,
> HDMI_PICTURE_ASPECT_16_9,
> + HDMI_PICTURE_ASPECT_64_27,
> + HDMI_PICTURE_ASPECT_256_135,
> HDMI_PICTURE_ASPECT_RESERVED,
> };
Where did you get these from? I'm asking because I sent this patch last year
(or at least I wrote it and we discussed it on IRC, since I can't find an email
archive link to it), and back at the time the picture aspect ratio was the big
question mark. My recollection is that CEA-861-F introduces these new picture
aspect ratios in the mode tables but never specifies their values. As a matter
of fact, the AVI infoframe where these values are used only has space for 4
values (none, 4:3, 16:9 and reserved).
Would you mind pointing me at the specification for these values?
Thanks,
Thierry
[Bug 95094] Flat uniform gets overwrites previous uniform
https://bugs.freedesktop.org/show_bug.cgi?id=95094 --- Comment #1 from Marek Olšák --- Hi, Do you have a small demo reproducing the issue? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/219ad566/attachment.html>
[Bug 95072] [wine] Rendering bug in GTA IV
https://bugs.freedesktop.org/show_bug.cgi?id=95072 --- Comment #2 from Marek Olšák --- Hi Sven, I can no longer reproduce this with Mesa git & LLVM git. Can you re-test? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/53025d69/attachment-0001.html>
[PATCH] Add virtio gpu driver.
On Mon, Mar 30, 2015 at 4:49 PM, Daniel Vetter wrote: > On Mon, Mar 30, 2015 at 02:23:47PM +0200, Gerd Hoffmann wrote: >> > > Signed-off-by: Dave Airlie >> > > Signed-off-by: Gerd Hoffmann >> > >> > Standard request from my side for new drm drivers (especially if they're >> > this simple): Can you please update the drivers to latest drm internal >> > interfaces, i.e. using universal planes and atomic? >> >> Up'n'running. Incremental patch: >> >> https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2d&id=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a >> >> v2 coming, but I'll go over the other reviews first. > > Looking good. Wrt pageflip the current MO is to handroll it in your > driver, common approach is to use the msm async commit implementation > msm_atomic_commit. The issue is simply that right now there's still no > useable generic vblank callback support (drm_irq.c is a mess) hence why > the core helpers don't support async flips yet. I guess I didn't do a good job at looking at your v2: Cursor is still using legacy interfaces and not a proper plane. Would be awesome if you could fix that up. Atomic drivers really shouldn't use the legacy cursor interfaces any more at all. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v3 7/7] [wip] virtio-gpu: add page flip support
On Fri, Oct 2, 2015 at 1:58 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
So I entirely missed this, but this isn't really how to implement
page_flip for an atomic driver. Working on some stuff and will hack up
a likely totally broken patch, but should be enough as guideline.
-Daniel
> ---
> drivers/gpu/drm/virtio/virtgpu_display.c | 49
> ++--
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> index c9c1427..f545913 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -125,6 +125,51 @@ static int virtio_gpu_crtc_cursor_move(struct drm_crtc
> *crtc,
> return 0;
> }
>
> +static int virtio_gpu_page_flip(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t flags)
> +{
> + struct virtio_gpu_device *vgdev = crtc->dev->dev_private;
> + struct virtio_gpu_output *output =
> + container_of(crtc, struct virtio_gpu_output, crtc);
> + struct drm_plane *plane = crtc->primary;
> + struct virtio_gpu_framebuffer *vgfb;
> + struct virtio_gpu_object *bo;
> + unsigned long irqflags;
> + uint32_t handle;
> +
> + plane->fb = fb;
> + vgfb = to_virtio_gpu_framebuffer(plane->fb);
> + bo = gem_to_virtio_gpu_obj(vgfb->obj);
> + handle = bo->hw_res_handle;
> +
> + DRM_DEBUG("handle 0x%x%s, crtc %dx%d\n", handle,
> + bo->dumb ? ", dumb" : "",
> + crtc->mode.hdisplay, crtc->mode.vdisplay);
> + if (bo->dumb) {
> + virtio_gpu_cmd_transfer_to_host_2d
> + (vgdev, handle, 0,
> +cpu_to_le32(crtc->mode.hdisplay),
> +cpu_to_le32(crtc->mode.vdisplay),
> +0, 0, NULL);
> + }
> + virtio_gpu_cmd_set_scanout(vgdev, output->index, handle,
> + crtc->mode.hdisplay,
> + crtc->mode.vdisplay, 0, 0);
> + virtio_gpu_cmd_resource_flush(vgdev, handle, 0, 0,
> + crtc->mode.hdisplay,
> + crtc->mode.vdisplay);
> +
> + if (event) {
> + spin_lock_irqsave(&crtc->dev->event_lock, irqflags);
> + drm_send_vblank_event(crtc->dev, -1, event);
> + spin_unlock_irqrestore(&crtc->dev->event_lock, irqflags);
> + }
> +
> + return 0;
> +}
> +
> static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
> .cursor_set2= virtio_gpu_crtc_cursor_set,
> .cursor_move= virtio_gpu_crtc_cursor_move,
> @@ -132,9 +177,7 @@ static const struct drm_crtc_funcs virtio_gpu_crtc_funcs
> = {
> .set_config = drm_atomic_helper_set_config,
> .destroy= drm_crtc_cleanup,
>
> -#if 0 /* not (yet) working without vblank support according to docs */
> - .page_flip = drm_atomic_helper_page_flip,
> -#endif
> + .page_flip = virtio_gpu_page_flip,
> .reset = drm_atomic_helper_crtc_reset,
> .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> --
> 1.8.3.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 12/20] drm/sun4i: Add bridge support
Hi Daniel, On Mon, May 16, 2016 at 03:12:15PM +0200, Daniel Vetter wrote: > On Mon, May 16, 2016 at 2:47 PM, Maxime Ripard > wrote: > > Our RGB bus can be either connected to a bridge or a panel. While the panel > > support was already there, the bridge was not. > > > > Fix that. > > > > Signed-off-by: Maxime Ripard > > For bridge support the only thing you need is to set > drm_encoder->bridge. Why do you need to hand-roll your own bridge > support? That pretty muchmeans you'll do it slightly differently (e.g. > you don't bother with a lot of the calls), which will make sharing > bridge drivers ever so harder. And also reviewing code, since using > shared code but slightly differently is really confusing. Because I overlooked it :) I'll change that. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/56a7f4a6/attachment.sig>
[PATCH 2/3] Define fb_open_adj_file()
On 05/24/2016 06:51 PM, Daniel Vetter wrote: > On Tue, May 24, 2016 at 6:28 PM, Max Staudt wrote: >> Hi Daniel, >> >> Thanks for the feedback! Comments below: >> >> >> On 05/23/2016 03:44 PM, Daniel Vetter wrote: >>> Do we _really_ care about fbdev mmap support so much that we want to add >>> more hacks all over the place (in each driver) to make it work? Given that >>> fbdev is officially in the "no more drivers" territory, I'm not sure we >>> want this either. >>> >>> The trouble is that fbdev mmap and kms dumb buffer mmap have different >>> semantics and so every driver needs to implement both if they're special >>> in any kind of way. >> >> DRM drivers already have fbdev hacks in place in order to implement both >> semantics. > > A lot of these fbdev hacks just disappeared since we've gained generic > defio support in 4.7. Is your assertion still true on 4.7? I really > don't want to grow hacks all over the place for something we should > try to solve once for everyone. > >> With this change, only bochsdrm(fb) will need to be touched, and if no >> further DRM driver implements fbdev support the way bochsdrmfb does, then no >> further driver will need this code. > > See above - we've grown meanwhile enough drm drivers which need > defio/special mmap support that there's now a pile of generic code and > refactoring in 4.7. It's not just bochsdrm by far. Also pretty much > every drm driver has their own pile of fbdev mmap hacks. > >> This patch set is really just about setting file->f_mapping in bochsdrmfb's >> fb_open(), so we finally get rid of the WARNING. >> >> We could also pass file as a third paramenter in fb_open(), changing the >> function definition in all fbdev drivers. I'm happy to change the patch in >> that way if this is preferred. >> >> >>> If we can't say no to fbdev mmap then I think we should implement >>> something that works for all drivers. I'm thinking of allocating just a >>> pile of pages in the fbdev emulation, and then copying them over using the >>> defio stuff we just merged for 4.7 into a dumb bo, plus calling dirtyfb. >>> Or at least something along those lines. Of couse just opt-in, in case the >>> driver can do a more traditional mmio fbdev mmapping. >> >> Well, the mmap code is standard fbdev and it's already there, so why not >> keep it? >> >> All this patch series does is allowing bochsdrmfb to work properly until >> fbdev support is finally removed from the kernel. > > I don't think we can remove fbdev in the next 10 years unfortunately. > It's deprecated insofar that no new drivers are accepted. Instead > proper kms drivers (with fbdev emulation) should be created. > > But that means another decade of new drivers in kms that need to add > hacks for fbdev mmap support, if we don't start to make this simpler > and avoid the need for such special cases. > > Anyway, that's kinda the high-level reasons why I jumped on this. I > think for the next steps in the discussion you need to take a look at > what's new in 4.7. And then we need to figure out what we need to add > to improve fbdev mmap further. > -Daniel Thanks, I'll look into it. Hm, sounds like it's best to ask the maintainer for advice, too. Gerd, could you please have a look at this thread? Thanks Max
[PATCH 04/14] gpu: ipu-v3: ipu-dmfc: Use static DMFC FIFO allocation mechanism
On Tue, May 24, 2016 at 10:23 PM, Philipp Zabel wrote: > Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying: >> For normal video modes such as 1280x720p at 60Hz and 1920x1080p at 60Hz, > > I'm very happy to drop all this code if possible, but the stated reason > is not good enough. Rather we should look at the worst case and make > sure that 2 FIFO slots are enough for 180 MPixel/s on i.MX5 and enough > for 240 MPixel/s on i.MX6. My stated reason is not accurate, perhaps. Sorry. IIRC, when I wrote this patch, I found we use 2 slots for all video modes with the dynamic allocation mechanism. That's why the static one comes up to mind. > According to the current heuristic (total bandwidth = clkrate * 4 pixels > with 8 slots) that should be true. I'm not sure what's the best allocation algorithm is. Hope that the static one can be accepted temporarily and some one may offer a better solution in future. Regards, Liu Ying > > regards > Philipp >
[git pull] drm for v4.7
On 25 May 2016 at 17:13, Linus Torvalds wrote: > On Wed, May 25, 2016 at 1:28 AM, Jani Nikula > wrote: >> >> There may be better ones out there, but Artem's "aiaiai" has some >> helpers [1] for diffing build logs, if you want something simple to >> integrate into existing scripts. > > It would be lovely to have some kind of warning detection, but quite > frankly, just doing a build and counting lines in 'stderr' from the > build and having some trigger for "oops, lots of new lines" would be > sufficient. > > So I don't think anything really fancy to diff build logs is > necessarily needed, although the people who then get the report about > "your merge causes lots of new warnings" might appreciate it. > The Intel 0-Day already checks when patch introduces new warnings, emailing the author and subsystem with the details. In this particular case, the patch was out the next day and I was silly enough not to ask Dave to include it in the same pull request. Regards, Emil
[PATCH] Add virtio gpu driver.
On 25 May 2016 at 17:40, Daniel Vetter wrote:
> On Mon, Mar 30, 2015 at 4:49 PM, Daniel Vetter wrote:
>> On Mon, Mar 30, 2015 at 02:23:47PM +0200, Gerd Hoffmann wrote:
>>> > > Signed-off-by: Dave Airlie
>>> > > Signed-off-by: Gerd Hoffmann
>>> >
>>> > Standard request from my side for new drm drivers (especially if they're
>>> > this simple): Can you please update the drivers to latest drm internal
>>> > interfaces, i.e. using universal planes and atomic?
>>>
>>> Up'n'running. Incremental patch:
>>>
>>> https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2d&id=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a
>>>
>>> v2 coming, but I'll go over the other reviews first.
>>
>> Looking good. Wrt pageflip the current MO is to handroll it in your
>> driver, common approach is to use the msm async commit implementation
>> msm_atomic_commit. The issue is simply that right now there's still no
>> useable generic vblank callback support (drm_irq.c is a mess) hence why
>> the core helpers don't support async flips yet.
>
> I guess I didn't do a good job at looking at your v2: Cursor is still
> using legacy interfaces and not a proper plane. Would be awesome if
> you could fix that up. Atomic drivers really shouldn't use the legacy
> cursor interfaces any more at all.
Wild idea:
Worth adding if (drm_core_check_feature(dev, DRIVER_ATOMIC) {
printf("abort abort"); return; }
style of checks for the legacy (preatomic) kms helpers ?
Or does it feel like an overkill ?
-Emil
[PATCH] drm/admgpu/powerplay/polaris: fix powertune table upload
From: Rex Zhu Exclude AVFS related fields when update powertune table to hw. The driver shouldn't set them directly. Signed-off-by: Rex Zhu Reviewed-by: Alex Deucher Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_powertune.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_powertune.c b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_powertune.c index 0b99ab3..ae96f14 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_powertune.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_powertune.c @@ -286,7 +286,7 @@ int polaris10_populate_pm_fuses(struct pp_hwmgr *hwmgr) if (polaris10_copy_bytes_to_smc(hwmgr->smumgr, pm_fuse_table_offset, (uint8_t *)&data->power_tune_table, - sizeof(struct SMU74_Discrete_PmFuses), data->sram_end)) + (sizeof(struct SMU74_Discrete_PmFuses) - 92), data->sram_end)) PP_ASSERT_WITH_CODE(false, "Attempt to download PmFuseTable Failed!", return -EINVAL); -- 2.5.5
[PATCH 05/14] drm/crtc_helper: Disable and reenable primary plane in drm_helper_crtc_mode_set
On Tue, May 24, 2016 at 6:57 PM, Daniel Vetter wrote: > On Tue, May 24, 2016 at 06:10:44PM +0800, Liu Ying wrote: >> Since CRTC has already been disabled in crtc_funcs->prepare(), it doesn't >> hurt >> to disable the primary plane in drm_helper_crtc_mode_set() before enabling it >> in drm_helper_crtc_mode_set_base(). This makes those who reject active plane >> update in plane_funcs->atomic_check() happy. > > How/where exactly do you blow up? >> >> Signed-off-by: Liu Ying >> --- >> drivers/gpu/drm/drm_crtc_helper.c | 9 + >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c >> b/drivers/gpu/drm/drm_crtc_helper.c >> index 79555d2..7fabcd7 100644 >> --- a/drivers/gpu/drm/drm_crtc_helper.c >> +++ b/drivers/gpu/drm/drm_crtc_helper.c >> @@ -1013,6 +1013,15 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, >> struct drm_display_mode *mod >> goto out; >> } >> >> + /* >> + * It doesn't hurt to disable primary plane here since crtc is off >> + * and we'll enable it again in drm_helper_crtc_mode_set_base() >> + * below soon. >> + */ >> + ret = drm_plane_helper_disable(crtc->primary); > > You can't call this helper from crtc helpers. drm_plane_helper_disable > assumes that a) you've already extracted universal plane support for the > primary plane by registering a proper drm_plane for it b) that you're > driver is at least partially using atomic hooks already. > > Both assumptions are wrong for all current users of this function. > drm_helper_crtc_mode_set() is purely a legacy helper for legecy drivers. Isn't the function a transitional helper, just as the kdoc claims? It looks all current users are sort of atomic users already. Regards, Liu Ying > -Daniel > >> + if (ret) >> + goto out; >> + >> swap(crtc->state, crtc_state); >> >> crtc_funcs->mode_set_nofb(crtc); >> -- >> 2.7.4 >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
[PATCH] drm/amdgpu/iceland: Set SC_PA_RASTER_CONFIG according to different RB enabled
From: Ken Wang fix the raster config setting for different iceland configs. Signed-off-by: Ken Wang Reviewed-by: Alex Deucher Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c index f19bab6..4d747ba 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c @@ -3974,11 +3974,15 @@ static int gfx_v8_0_cp_gfx_start(struct amdgpu_device *adev) amdgpu_ring_write(ring, 0x3a00161a); amdgpu_ring_write(ring, 0x002e); break; - case CHIP_TOPAZ: case CHIP_CARRIZO: amdgpu_ring_write(ring, 0x0002); amdgpu_ring_write(ring, 0x); break; + case CHIP_TOPAZ: + amdgpu_ring_write(ring, adev->gfx.config.num_rbs == 1 ? + 0x : 0x0002); + amdgpu_ring_write(ring, 0x); + break; case CHIP_STONEY: amdgpu_ring_write(ring, 0x); amdgpu_ring_write(ring, 0x); -- 2.5.5
[PATCH 10/14] drm/atomic-helper: Disable planes when suspending
On Tue, May 24, 2016 at 7:00 PM, Daniel Vetter wrote:
> On Tue, May 24, 2016 at 06:10:49PM +0800, Liu Ying wrote:
>> We should disable planes explicitly when suspending.
>> Especially, this is meaningful for those display controllers which
>> don't support active planes without relevant CRTCs being enabled.
>>
>> Signed-off-by: Liu Ying
>
> Recommended way is to call drm_atomic_helper_disable_planes_on_crtc in
> your crtc's ->disable() callback if your hw needs this. This is a general
> problem (test e.g. dpms), not just an issue in suspend code.
It looks legacy fbdev unblank operation fails after call that function
in ->disable(). I made the change on top of this patch set.
If you have any idea, please help out here.
Regards,
Liu Ying
>
> Also unsetting the planes from state has a semantic meaning: It unpins the
> backing storage, which is definitely not what we want for suspend/resume.
> -Daniel
>
>> ---
>> drivers/gpu/drm/drm_atomic_helper.c | 18 +-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index 4befe25..5331d95 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1967,7 +1967,7 @@ commit:
>> *
>> * Loops through all connectors, finding those that aren't turned off and
>> then
>> * turns them off by setting their DPMS mode to OFF and deactivating the
>> CRTC
>> - * that they are connected to.
>> + * that they are connected to. The relevant planes are deactivated as well.
>> *
>> * This is used for example in suspend/resume to disable all currently
>> active
>> * functions when suspending.
>> @@ -1997,6 +1997,7 @@ int drm_atomic_helper_disable_all(struct drm_device
>> *dev,
>> drm_for_each_connector(conn, dev) {
>> struct drm_crtc *crtc = conn->state->crtc;
>> struct drm_crtc_state *crtc_state;
>> + struct drm_plane *plane;
>>
>> if (!crtc || conn->dpms != DRM_MODE_DPMS_ON)
>> continue;
>> @@ -2008,6 +2009,21 @@ int drm_atomic_helper_disable_all(struct drm_device
>> *dev,
>> }
>>
>> crtc_state->active = false;
>> +
>> + drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
>> + struct drm_plane_state *plane_state;
>> +
>> + plane_state = drm_atomic_get_plane_state(state, plane);
>> + if (IS_ERR(plane_state)) {
>> + err = PTR_ERR(plane_state);
>> + goto free;
>> + }
>> +
>> + err = drm_atomic_set_crtc_for_plane(plane_state, NULL);
>> + if (err != 0)
>> + goto free;
>> + drm_atomic_set_fb_for_plane(plane_state, NULL);
>> + }
>> }
>>
>> err = drm_atomic_commit(state);
>> --
>> 2.7.4
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
[PATCH 5/5] drm/amd/powerplay: add mclk OD(overdrive) support for Polaris10
From: Eric Huang
The maximum OD percentage is 20.
Reviewed-by: Alex Deucher
Signed-off-by: Eric Huang
Signed-off-by: Alex Deucher
---
.../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 43 ++
1 file changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
index f91be22..9be0b66 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
@@ -4946,6 +4946,47 @@ static int polaris10_set_sclk_od(struct pp_hwmgr *hwmgr,
uint32_t value)
return 0;
}
+static int polaris10_get_mclk_od(struct pp_hwmgr *hwmgr)
+{
+ struct polaris10_hwmgr *data = (struct polaris10_hwmgr
*)(hwmgr->backend);
+ struct polaris10_single_dpm_table *mclk_table =
&(data->dpm_table.mclk_table);
+ struct polaris10_single_dpm_table *golden_mclk_table =
+ &(data->golden_dpm_table.mclk_table);
+ int value;
+
+ value = (mclk_table->dpm_levels[mclk_table->count - 1].value -
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value) *
+ 100 /
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return value;
+}
+
+static int polaris10_set_mclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
+{
+ struct polaris10_hwmgr *data = (struct polaris10_hwmgr
*)(hwmgr->backend);
+ struct polaris10_single_dpm_table *golden_mclk_table =
+ &(data->golden_dpm_table.mclk_table);
+ struct pp_power_state *ps;
+ struct polaris10_power_state *polaris10_ps;
+
+ if (value > 20)
+ value = 20;
+
+ ps = hwmgr->request_ps;
+
+ if (ps == NULL)
+ return -EINVAL;
+
+ polaris10_ps = cast_phw_polaris10_power_state(&ps->hardware);
+
+ polaris10_ps->performance_levels[polaris10_ps->performance_level_count
- 1].memory_clock =
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value *
+ value / 100 +
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return 0;
+}
static const struct pp_hwmgr_func polaris10_hwmgr_funcs = {
.backend_init = &polaris10_hwmgr_backend_init,
.backend_fini = &polaris10_hwmgr_backend_fini,
@@ -4991,6 +5032,8 @@ static const struct pp_hwmgr_func polaris10_hwmgr_funcs =
{
.enable_per_cu_power_gating = polaris10_phm_enable_per_cu_power_gating,
.get_sclk_od = polaris10_get_sclk_od,
.set_sclk_od = polaris10_set_sclk_od,
+ .get_mclk_od = polaris10_get_mclk_od,
+ .set_mclk_od = polaris10_set_mclk_od,
};
int polaris10_hwmgr_init(struct pp_hwmgr *hwmgr)
--
2.5.5
[PATCH 4/5] drm/amd/powerplay: add mclk OD(overdrive) support for Fiji
From: Eric Huang
The maximum OD percentage is 20.
Reviewed-by: Alex Deucher
Signed-off-by: Eric Huang
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 44
1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
index 76beaf8..f5df116 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
@@ -5316,6 +5316,48 @@ static int fiji_set_sclk_od(struct pp_hwmgr *hwmgr,
uint32_t value)
return 0;
}
+static int fiji_get_mclk_od(struct pp_hwmgr *hwmgr)
+{
+ struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
+ struct fiji_single_dpm_table *mclk_table =
&(data->dpm_table.mclk_table);
+ struct fiji_single_dpm_table *golden_mclk_table =
+ &(data->golden_dpm_table.mclk_table);
+ int value;
+
+ value = (mclk_table->dpm_levels[mclk_table->count - 1].value -
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value) *
+ 100 /
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return value;
+}
+
+static int fiji_set_mclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
+{
+ struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
+ struct fiji_single_dpm_table *golden_mclk_table =
+ &(data->golden_dpm_table.mclk_table);
+ struct pp_power_state *ps;
+ struct fiji_power_state *fiji_ps;
+
+ if (value > 20)
+ value = 20;
+
+ ps = hwmgr->request_ps;
+
+ if (ps == NULL)
+ return -EINVAL;
+
+ fiji_ps = cast_phw_fiji_power_state(&ps->hardware);
+
+ fiji_ps->performance_levels[fiji_ps->performance_level_count -
1].memory_clock =
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value *
+ value / 100 +
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return 0;
+}
+
static const struct pp_hwmgr_func fiji_hwmgr_funcs = {
.backend_init = &fiji_hwmgr_backend_init,
.backend_fini = &fiji_hwmgr_backend_fini,
@@ -5359,6 +5401,8 @@ static const struct pp_hwmgr_func fiji_hwmgr_funcs = {
.print_clock_levels = fiji_print_clock_levels,
.get_sclk_od = fiji_get_sclk_od,
.set_sclk_od = fiji_set_sclk_od,
+ .get_mclk_od = fiji_get_mclk_od,
+ .set_mclk_od = fiji_set_mclk_od,
};
int fiji_hwmgr_init(struct pp_hwmgr *hwmgr)
--
2.5.5
[PATCH 3/5] drm/amd/powerplay: add mclk OD(overdrive) support for Tonga
From: Eric Huang
The maximum OD percentage is 20.
Reviewed-by: Alex Deucher
Signed-off-by: Eric Huang
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 44 +++
1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
index 01bf983..3e50b29 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
@@ -6236,6 +6236,48 @@ static int tonga_set_sclk_od(struct pp_hwmgr *hwmgr,
uint32_t value)
return 0;
}
+static int tonga_get_mclk_od(struct pp_hwmgr *hwmgr)
+{
+ struct tonga_hwmgr *data = (struct tonga_hwmgr *)(hwmgr->backend);
+ struct tonga_single_dpm_table *mclk_table =
&(data->dpm_table.mclk_table);
+ struct tonga_single_dpm_table *golden_mclk_table =
+ &(data->golden_dpm_table.mclk_table);
+ int value;
+
+ value = (mclk_table->dpm_levels[mclk_table->count - 1].value -
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value) *
+ 100 /
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return value;
+}
+
+static int tonga_set_mclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
+{
+ struct tonga_hwmgr *data = (struct tonga_hwmgr *)(hwmgr->backend);
+ struct tonga_single_dpm_table *golden_mclk_table =
+ &(data->golden_dpm_table.mclk_table);
+ struct pp_power_state *ps;
+ struct tonga_power_state *tonga_ps;
+
+ if (value > 20)
+ value = 20;
+
+ ps = hwmgr->request_ps;
+
+ if (ps == NULL)
+ return -EINVAL;
+
+ tonga_ps = cast_phw_tonga_power_state(&ps->hardware);
+
+ tonga_ps->performance_levels[tonga_ps->performance_level_count -
1].memory_clock =
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value *
+ value / 100 +
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return 0;
+}
+
static const struct pp_hwmgr_func tonga_hwmgr_funcs = {
.backend_init = &tonga_hwmgr_backend_init,
.backend_fini = &tonga_hwmgr_backend_fini,
@@ -6280,6 +6322,8 @@ static const struct pp_hwmgr_func tonga_hwmgr_funcs = {
.print_clock_levels = tonga_print_clock_levels,
.get_sclk_od = tonga_get_sclk_od,
.set_sclk_od = tonga_set_sclk_od,
+ .get_mclk_od = tonga_get_mclk_od,
+ .set_mclk_od = tonga_set_mclk_od,
};
int tonga_hwmgr_init(struct pp_hwmgr *hwmgr)
--
2.5.5
[PATCH 2/5] drm/amdgpu: add mclk OD(overdrive) support for CI
From: Eric Huang
The maximum OD percentage is 20.
Reviewed-by: Alex Deucher
Signed-off-by: Eric Huang
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/ci_dpm.c | 36
1 file changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
index fb968dd..671d032 100644
--- a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
@@ -6551,6 +6551,40 @@ static int ci_dpm_set_sclk_od(struct amdgpu_device
*adev, uint32_t value)
return 0;
}
+static int ci_dpm_get_mclk_od(struct amdgpu_device *adev)
+{
+ struct ci_power_info *pi = ci_get_pi(adev);
+ struct ci_single_dpm_table *mclk_table = &(pi->dpm_table.mclk_table);
+ struct ci_single_dpm_table *golden_mclk_table =
+ &(pi->golden_dpm_table.mclk_table);
+ int value;
+
+ value = (mclk_table->dpm_levels[mclk_table->count - 1].value -
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value) *
+ 100 /
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return value;
+}
+
+static int ci_dpm_set_mclk_od(struct amdgpu_device *adev, uint32_t value)
+{
+ struct ci_power_info *pi = ci_get_pi(adev);
+ struct ci_ps *ps = ci_get_ps(adev->pm.dpm.requested_ps);
+ struct ci_single_dpm_table *golden_mclk_table =
+ &(pi->golden_dpm_table.mclk_table);
+
+ if (value > 20)
+ value = 20;
+
+ ps->performance_levels[ps->performance_level_count - 1].mclk =
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value *
+ value / 100 +
+ golden_mclk_table->dpm_levels[golden_mclk_table->count
- 1].value;
+
+ return 0;
+}
+
const struct amd_ip_funcs ci_dpm_ip_funcs = {
.name = "ci_dpm",
.early_init = ci_dpm_early_init,
@@ -6589,6 +6623,8 @@ static const struct amdgpu_dpm_funcs ci_dpm_funcs = {
.force_clock_level = ci_dpm_force_clock_level,
.get_sclk_od = ci_dpm_get_sclk_od,
.set_sclk_od = ci_dpm_set_sclk_od,
+ .get_mclk_od = ci_dpm_get_mclk_od,
+ .set_mclk_od = ci_dpm_set_mclk_od,
};
static void ci_dpm_set_dpm_funcs(struct amdgpu_device *adev)
--
2.5.5
[PATCH 1/5] drm/amdgpu: add the common code to support mclk OD
From: Eric Huang
This implements mclk OverDrive(OD) through sysfs.
The new entry pp_mclk_od is read/write. The value of input/output
is an integer of the overclocking percentage.
Reviewed-by: Alex Deucher
Signed-off-by: Eric Huang
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 55 +++
drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 40 +
drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h | 2 +
drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 2 +
5 files changed, 107 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index bfc994bc..ab122cc4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1563,6 +1563,8 @@ struct amdgpu_dpm_funcs {
int (*print_clock_levels)(struct amdgpu_device *adev, enum
pp_clock_type type, char *buf);
int (*get_sclk_od)(struct amdgpu_device *adev);
int (*set_sclk_od)(struct amdgpu_device *adev, uint32_t value);
+ int (*get_mclk_od)(struct amdgpu_device *adev);
+ int (*set_mclk_od)(struct amdgpu_device *adev, uint32_t value);
};
struct amdgpu_dpm {
@@ -2344,6 +2346,12 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
#define amdgpu_dpm_set_sclk_od(adev, value) \
(adev)->powerplay.pp_funcs->set_sclk_od((adev)->powerplay.pp_handle,
value)
+#define amdgpu_dpm_get_mclk_od(adev) \
+ ((adev)->powerplay.pp_funcs->get_mclk_od((adev)->powerplay.pp_handle))
+
+#define amdgpu_dpm_set_mclk_od(adev, value) \
+ ((adev)->powerplay.pp_funcs->set_mclk_od((adev)->powerplay.pp_handle,
value))
+
#define amdgpu_dpm_dispatch_task(adev, event_id, input, output)
\
(adev)->powerplay.pp_funcs->dispatch_tasks((adev)->powerplay.pp_handle,
(event_id), (input), (output))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index ebb99fd..7f84154 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -529,6 +529,52 @@ fail:
return count;
}
+static ssize_t amdgpu_get_pp_mclk_od(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);
+ struct amdgpu_device *adev = ddev->dev_private;
+ uint32_t value = 0;
+
+ if (adev->pp_enabled)
+ value = amdgpu_dpm_get_mclk_od(adev);
+ else if (adev->pm.funcs->get_mclk_od)
+ value = adev->pm.funcs->get_mclk_od(adev);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static ssize_t amdgpu_set_pp_mclk_od(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);
+ struct amdgpu_device *adev = ddev->dev_private;
+ int ret;
+ long int value;
+
+ ret = kstrtol(buf, 0, &value);
+
+ if (ret) {
+ count = -EINVAL;
+ goto fail;
+ }
+
+ if (adev->pp_enabled) {
+ amdgpu_dpm_set_mclk_od(adev, (uint32_t)value);
+ amdgpu_dpm_dispatch_task(adev,
AMD_PP_EVENT_READJUST_POWER_STATE, NULL, NULL);
+ } else if (adev->pm.funcs->set_mclk_od) {
+ adev->pm.funcs->set_mclk_od(adev, (uint32_t)value);
+ adev->pm.dpm.current_ps = adev->pm.dpm.boot_ps;
+ amdgpu_pm_compute_clocks(adev);
+ }
+
+fail:
+ return count;
+}
+
static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state,
amdgpu_set_dpm_state);
static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
amdgpu_get_dpm_forced_performance_level,
@@ -553,6 +599,9 @@ static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
static DEVICE_ATTR(pp_sclk_od, S_IRUGO | S_IWUSR,
amdgpu_get_pp_sclk_od,
amdgpu_set_pp_sclk_od);
+static DEVICE_ATTR(pp_mclk_od, S_IRUGO | S_IWUSR,
+ amdgpu_get_pp_mclk_od,
+ amdgpu_set_pp_mclk_od);
static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
struct device_attribute *attr,
@@ -1193,6 +1242,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
DRM_ERROR("failed to create device file pp_sclk_od\n");
return ret;
}
+ ret = device_create_file(adev->dev, &dev_attr_pp_mclk_od);
+ if (ret) {
+ DRM_ERROR("failed to create device file pp_mclk_od\n");
+ return ret;
+ }
ret = amdgpu_debugfs_pm_init(adev);
if (ret) {
@@ -1221,6 +1275,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
device_remove_file(adev->dev, &dev_attr_pp_dpm_mclk);
device_remove_file(adev->dev, &dev_attr_pp_dpm_pcie);
[PATCH 0/5] Add mclk OD (OverDrive) support
This patch set adds mclk (memory clock) overclocking support to amdgpu for dGPUs. The interface is the same as for sclk (engine clock), a percentage from 0 to 20. Eric Huang (5): drm/amdgpu: add the common code to support mclk OD drm/amdgpu: add mclk OD(overdrive) support for CI drm/amd/powerplay: add mclk OD(overdrive) support for Tonga drm/amd/powerplay: add mclk OD(overdrive) support for Fiji drm/amd/powerplay: add mclk OD(overdrive) support for Polaris10 drivers/gpu/drm/amd/amdgpu/amdgpu.h| 8 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 55 ++ drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 36 ++ drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 40 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 44 + .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 43 + drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 44 + drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h | 2 + drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 2 + 9 files changed, 274 insertions(+) -- 2.5.5
[PATCH 13/14] drm/imx: atomic phase 3 step 4: Use generic atomic page flip
On Tue, May 24, 2016 at 7:11 PM, Daniel Vetter wrote:
> On Tue, May 24, 2016 at 06:10:52PM +0800, Liu Ying wrote:
>> To support generic atomic page flip, this patch customizes ->atomic_commit
>> for async commits.
>
> It's now called nonblocking instead of async. Please run s/async/nonblock/
> over this patch.
Ok. Will do.
Regards,
Liu Ying
> -Daniel
>
>>
>> Signed-off-by: Liu Ying
>> ---
>> drivers/gpu/drm/imx/imx-drm-core.c | 137 +++-
>> drivers/gpu/drm/imx/ipuv3-crtc.c | 156
>> ++---
>> 2 files changed, 144 insertions(+), 149 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
>> b/drivers/gpu/drm/imx/imx-drm-core.c
>> index 2fa04a0..cb521cb 100644
>> --- a/drivers/gpu/drm/imx/imx-drm-core.c
>> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
>> @@ -15,10 +15,14 @@
>> */
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -48,6 +52,14 @@ struct imx_drm_device {
>> struct imx_drm_crtc {
>> struct drm_crtc *crtc;
>> struct imx_drm_crtc_helper_funcsimx_drm_helper_funcs;
>> + wait_queue_head_t commit_wait;
>> + boolcommit_pending;
>> +};
>> +
>> +struct imx_drm_commit {
>> + struct work_struct work;
>> + struct drm_device *dev;
>> + struct drm_atomic_state *state;
>> };
>>
>> #if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
>> @@ -168,11 +180,132 @@ static void imx_drm_output_poll_changed(struct
>> drm_device *drm)
>> drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
>> }
>>
>> +static void imx_drm_atomic_complete(struct imx_drm_commit *commit, bool
>> async)
>> +{
>> + struct drm_device *dev = commit->dev;
>> + struct imx_drm_device *imxdrm = dev->dev_private;
>> + struct imx_drm_crtc *imx_crtc;
>> + struct drm_atomic_state *old_state = commit->state;
>> + struct drm_crtc *crtc;
>> + struct drm_crtc_state *old_crtc_state;
>> + struct drm_plane_state *plane_state;
>> + struct drm_gem_cma_object *cma_obj;
>> + struct fence *excl;
>> + unsigned shared_count;
>> + struct fence **shared;
>> + unsigned int i, j;
>> + int ret;
>> +
>> + /* Wait for fences. */
>> + for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
>> + if (crtc->state->event) {
>> + plane_state = crtc->primary->state;
>> + cma_obj = drm_fb_cma_get_gem_obj(plane_state->fb, 0);
>> + if (cma_obj->base.dma_buf) {
>> + ret = reservation_object_get_fences_rcu(
>> + cma_obj->base.dma_buf->resv, &excl,
>> + &shared_count, &shared);
>> + if (unlikely(ret))
>> + DRM_ERROR("failed to get fences "
>> + "for buffer\n");
>> +
>> + if (excl) {
>> + fence_wait(excl, false);
>> + fence_put(excl);
>> + }
>> + for (j = 0; j < shared_count; i++) {
>> + fence_wait(shared[j], false);
>> + fence_put(shared[j]);
>> + }
>> + }
>> + }
>> + }
>> +
>> + /* Apply the atomic update. */
>> + drm_atomic_helper_commit_modeset_disables(dev, old_state);
>> + drm_atomic_helper_commit_modeset_enables(dev, old_state);
>> + drm_atomic_helper_commit_planes(dev, old_state, false);
>> + drm_atomic_helper_wait_for_vblanks(dev, old_state);
>> + drm_atomic_helper_cleanup_planes(dev, old_state);
>> +
>> + if (async)
>> + for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
>> + imx_crtc = imxdrm->crtc[i];
>> +
>> + /* Complete the commit, wake up any waiter. */
>> + spin_lock(&imx_crtc->commit_wait.lock);
>> + imx_crtc->commit_pending = false;
>> + wake_up_all_locked(&imx_crtc->commit_wait);
>> + spin_unlock(&imx_crtc->commit_wait.lock);
>> + }
>> +
>> + drm_atomic_state_free(old_state);
>> +
>> + kfree(commit);
>> +}
>> +
>> +static void imx_drm_atomic_work(struct work_struct *work)
>> +{
>> + struct imx_drm_commit *commit =
>> + container_of(work, struct imx_drm_commit, work);
>> +
>> + imx_drm_atomic_complete(commit, true);
>> +}
>> +
>> +static int imx_drm_atomic_commit(struct drm_device *dev,
>> + struct drm_atomic_state *state, bool async)
>> +{
>> + struct imx_drm_device *imxdrm =
[PATCH 00/14] imx drm atomic mode setting conversion
On Tue, May 24, 2016 at 10:19 PM, Philipp Zabel wrote: > Hi Liu, > > Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying: >> Hi, >> >> This patch set converts imx drm into atomic mode setting. >> It takes 3 phases to achieve the goal as recommended. >> This patch set may apply to Philipp Zabel's open git branch imx-drm/fixes. > > Thank you for the rework! Could I ask you to rebase this onto > imx-drm/next? With some comments addressed, I have a local version which is based on imx-drm/next. Will send it out after it is mature. > I have applied the first two patches, and I have some superficial > comments for some of the later patches at this time. Still have to look > at the rest in detail. Thanks for the review. Regards, Liu Ying > > best regards > Philipp >
[PATCH 03/14] drm: imx: ipuv3 plane: Check different types of plane separately
On Tue, May 24, 2016 at 10:20 PM, Philipp Zabel
wrote:
> Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying:
>> The IPUv3 primary plane doesn't support partial off screen.
>> So, this patch separates plane check logics for primary plane and overlay
>> plane and adds more limitations on the primary plane.
>>
>> Signed-off-by: Liu Ying
>> ---
>> drivers/gpu/drm/imx/ipuv3-plane.c | 72
>> ++-
>> 1 file changed, 41 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
>> b/drivers/gpu/drm/imx/ipuv3-plane.c
>> index e6ec8eb..8f91b2e 100644
>> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
>> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
>> @@ -190,44 +190,54 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane,
>> struct drm_crtc *crtc,
>> uint32_t src_x, uint32_t src_y,
>> uint32_t src_w, uint32_t src_h, bool interlaced)
>> {
>> - struct device *dev = ipu_plane->base.dev->dev;
>> + struct drm_plane plane = ipu_plane->base;
>
> Why make a copy of the drm_plane here?
>
>> + struct device *dev = plane.dev->dev;
>> int ret;
>>
>> /* no scaling */
>> if (src_w != crtc_w || src_h != crtc_h)
>> return -EINVAL;
>>
>> - /* clip to crtc bounds */
>> - if (crtc_x < 0) {
>> - if (-crtc_x > crtc_w)
>> + if (plane.type == DRM_PLANE_TYPE_PRIMARY) {
>> + /* full plane doesn't support partial off screen */
>> + if (crtc_x || crtc_y || crtc_w != mode->hdisplay ||
>> + crtc_h != mode->vdisplay)
>
> As long as the requested plane is large enough to cover the whole base
> plane, we can fix the crtc_x/y/w/h up by clipping to the base plane
> boundaries. There is no need to return -EINVAL here as long as the IDMAC
> is capable to start reading at src_x/y = -crtc_x/y.
For the primary plane, we really need to look at mode->v/hdisplay
to make sure no partial off screen happens(plane partial off CRTC).
The clip is tricky, so for primary plane, I just take the simple way
to avoid the clip.
Eventually, we'll use atomic check to simply accept or reject
the user's request instead of changing the state to do clip.
This makes the driver simpler.
Regards,
Liu Ying
>
> regards
> Philipp
>
[PATCH] drm/amdgpu: add powercontainment module parameter
From: Huang Rui
This patch makes powercontainment feature configurable. Currently, the
powercontainment is not very stable, so add a module parameter to
enable/disable it via user mode.
Signed-off-by: Huang Rui
Reviewed-by: Alex Deucher
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4
drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c | 1 +
drivers/gpu/drm/amd/powerplay/hwmgr/fiji_powertune.c | 15 ---
drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 1 +
drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 9 +++--
drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h | 1 +
drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 +
8 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e72104b..bfc994bc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -85,6 +85,7 @@ extern int amdgpu_vm_debug;
extern int amdgpu_sched_jobs;
extern int amdgpu_sched_hw_submission;
extern int amdgpu_powerplay;
+extern int amdgpu_powercontainment;
extern unsigned amdgpu_pcie_gen_cap;
extern unsigned amdgpu_pcie_lane_cap;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f888c01..f122703 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -82,6 +82,7 @@ int amdgpu_exp_hw_support = 0;
int amdgpu_sched_jobs = 32;
int amdgpu_sched_hw_submission = 2;
int amdgpu_powerplay = -1;
+int amdgpu_powercontainment = 1;
unsigned amdgpu_pcie_gen_cap = 0;
unsigned amdgpu_pcie_lane_cap = 0;
@@ -160,6 +161,9 @@ module_param_named(sched_hw_submission,
amdgpu_sched_hw_submission, int, 0444);
#ifdef CONFIG_DRM_AMD_POWERPLAY
MODULE_PARM_DESC(powerplay, "Powerplay component (1 = enable, 0 = disable, -1
= auto (default))");
module_param_named(powerplay, amdgpu_powerplay, int, 0444);
+
+MODULE_PARM_DESC(powercontainment, "Power Containment (1 = enable (default), 0
= disable)");
+module_param_named(powercontainment, amdgpu_powercontainment, int, 0444);
#endif
MODULE_PARM_DESC(pcie_gen_cap, "PCIE Gen Caps (0: autodetect (default))");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c
index 6bd961f..cfbb360 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c
@@ -52,6 +52,7 @@ static int amdgpu_powerplay_init(struct amdgpu_device *adev)
pp_init->chip_family = adev->family;
pp_init->chip_id = adev->asic_type;
pp_init->device = amdgpu_cgs_create_device(adev);
+ pp_init->powercontainment_enabled = amdgpu_powercontainment;
ret = amd_powerplay_init(pp_init, amd_pp);
kfree(pp_init);
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_powertune.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_powertune.c
index db23a40..572729b 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_powertune.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_powertune.c
@@ -73,17 +73,18 @@ void fiji_initialize_power_tune_defaults(struct pp_hwmgr
*hwmgr)
if (!tmp) {
phm_cap_set(hwmgr->platform_descriptor.platformCaps,
- PHM_PlatformCaps_PowerContainment);
-
- phm_cap_set(hwmgr->platform_descriptor.platformCaps,
PHM_PlatformCaps_CAC);
fiji_hwmgr->fast_watermark_threshold = 100;
- tmp = 1;
- fiji_hwmgr->enable_dte_feature = tmp ? false : true;
- fiji_hwmgr->enable_tdc_limit_feature = tmp ? true : false;
- fiji_hwmgr->enable_pkg_pwr_tracking_feature = tmp ? true :
false;
+ if (hwmgr->powercontainment_enabled) {
+ phm_cap_set(hwmgr->platform_descriptor.platformCaps,
+ PHM_PlatformCaps_PowerContainment);
+ tmp = 1;
+ fiji_hwmgr->enable_dte_feature = tmp ? false : true;
+ fiji_hwmgr->enable_tdc_limit_feature = tmp ? true :
false;
+ fiji_hwmgr->enable_pkg_pwr_tracking_feature = tmp ?
true : false;
+ }
}
}
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
index 1c48917..404eedc 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
@@ -58,6 +58,7 @@ int hwmgr_init(struct amd_pp_init *pp_init, struct
pp_instance *handle)
hwmgr->hw_revision = pp_init->rev_id;
hwmgr->usec_timeout = AMD_MAX_USEC_TIMEOUT;
hwmgr->power_source = PP_PowerSource_AC;
+ hwmgr->powercontainment_enabled = pp_init->powerco
[PATCH 2/2] drm/amdgpu/gfx6: Assign num_rbs
From: Ken Wang
This got missed in the rebase as the code changed.
Signed-off-by: Ken Wang
Reviewed-by: Alex Deucher
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 561d67a..8b9f432 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -944,6 +944,7 @@ static void gfx_v6_0_setup_rb(struct amdgpu_device *adev,
adev->gfx.config.backend_enable_mask = enabled_rbs;
DRM_INFO("amdgpu: enabled_rbs %d \n", enabled_rbs);
+ adev->gfx.config.num_rbs = hweight32(enabled_rbs);
for (i = 0; i < se_num; i++) {
gfx_v6_0_select_se_sh(adev, i, 0x);
--
2.5.5
[PATCH 1/2] drm/amdgpu: Add SI Family information
From: Ken Wang
Signed-off-by: Ken Wang
Reviewed-by: Alex Deucher
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
include/uapi/drm/amdgpu_drm.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index d269740..274473d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1169,6 +1169,7 @@ static int amdgpu_early_init(struct amdgpu_device *adev)
case CHIP_PITCAIRN:
case CHIP_OLAND:
case CHIP_HAINAN:
+ adev->family = AMDGPU_FAMILY_SI;
r = si_set_ip_blocks(adev);
if (r)
return r;
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 453a76a..b473b53 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -637,6 +637,7 @@ struct drm_amdgpu_info_hw_ip {
* Supported GPU families
*/
#define AMDGPU_FAMILY_UNKNOWN 0
+#define AMDGPU_FAMILY_SI 110 /* Hainan, Oland, Verde,
Pitcairn, Tahiti */
#define AMDGPU_FAMILY_CI 120 /* Bonaire, Hawaii */
#define AMDGPU_FAMILY_KV 125 /* Kaveri, Kabini, Mullins
*/
#define AMDGPU_FAMILY_VI 130 /* Iceland, Tonga */
--
2.5.5
[PATCH 8/8] drm/amdgpu: fix and cleanup job destruction
From: Christian König
Remove the job reference counting and just properly destroy it from a
work item which blocks on any potential running timeout handler.
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 18 +++
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 46 +++
drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 25 +++
5 files changed, 30 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ec42dd5..e72104b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -755,7 +755,6 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev,
unsigned size,
struct amdgpu_job **job);
void amdgpu_job_free(struct amdgpu_job *job);
-void amdgpu_job_free_func(struct kref *refcount);
int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
struct amd_sched_entity *entity, void *owner,
struct fence **f);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d8cac31..a3d7d13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -839,8 +839,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
p->job = NULL;
r = amd_sched_job_init(&job->base, &ring->sched,
- entity, amdgpu_job_free_func,
- p->filp, &fence);
+ entity, p->filp, &fence);
if (r) {
amdgpu_job_free(job);
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 32132f2..34cd971 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -28,12 +28,6 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
-static void amdgpu_job_free_handler(struct work_struct *ws)
-{
- struct amdgpu_job *job = container_of(ws, struct amdgpu_job,
base.work_free_job);
- amd_sched_job_put(&job->base);
-}
-
static void amdgpu_job_timedout(struct amd_sched_job *s_job)
{
struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base);
@@ -42,8 +36,6 @@ static void amdgpu_job_timedout(struct amd_sched_job *s_job)
job->base.sched->name,
atomic_read(&job->ring->fence_drv.last_seq),
job->ring->fence_drv.sync_seq);
-
- amd_sched_job_put(&job->base);
}
int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
@@ -64,7 +56,6 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned
num_ibs,
(*job)->vm = vm;
(*job)->ibs = (void *)&(*job)[1];
(*job)->num_ibs = num_ibs;
- INIT_WORK(&(*job)->base.work_free_job, amdgpu_job_free_handler);
amdgpu_sync_create(&(*job)->sync);
@@ -103,9 +94,10 @@ static void amdgpu_job_free_resources(struct amdgpu_job
*job)
amdgpu_sync_free(&job->sync);
}
-void amdgpu_job_free_func(struct kref *refcount)
+void amdgpu_job_free_cb(struct amd_sched_job *s_job)
{
- struct amdgpu_job *job = container_of(refcount, struct amdgpu_job,
base.refcount);
+ struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base);
+
kfree(job);
}
@@ -126,8 +118,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct
amdgpu_ring *ring,
if (!f)
return -EINVAL;
- r = amd_sched_job_init(&job->base, &ring->sched,
- entity, amdgpu_job_free_func, owner, &fence);
+ r = amd_sched_job_init(&job->base, &ring->sched, entity, owner, &fence);
if (r)
return r;
@@ -198,4 +189,5 @@ const struct amd_sched_backend_ops amdgpu_sched_ops = {
.dependency = amdgpu_job_dependency,
.run_job = amdgpu_job_run,
.timedout_job = amdgpu_job_timedout,
+ .free_job = amdgpu_job_free_cb
};
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index cb56d90..2425172 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -319,19 +319,13 @@ static bool amd_sched_entity_in(struct amd_sched_job
*sched_job)
return added;
}
-static void amd_sched_free_job(struct fence *f, struct fence_cb *cb) {
- struct amd_sched_job *job = container_of(cb, struct amd_sched_job,
-cb_free_job);
-
- schedule_work(&job->work_free_job);
-}
-
/* job_finish is called after hw fence signaled, and
* the job had already been deleted from ring_mirror_list
*/
-static void amd_sched_job_finish(str
[PATCH 7/8] drm/amdgpu: move locking into the functions who need it
From: Christian König
Otherwise the locking becomes rather confusing.
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index f2ed8c5..cb56d90 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -333,7 +333,11 @@ static void amd_sched_job_finish(struct amd_sched_job
*s_job)
{
struct amd_sched_job *next;
struct amd_gpu_scheduler *sched = s_job->sched;
+ unsigned long flags;
+ /* remove job from ring_mirror_list */
+ spin_lock_irqsave(&sched->job_list_lock, flags);
+ list_del_init(&s_job->node);
if (sched->timeout != MAX_SCHEDULE_TIMEOUT) {
if (cancel_delayed_work(&s_job->work_tdr))
amd_sched_job_put(s_job);
@@ -347,12 +351,16 @@ static void amd_sched_job_finish(struct amd_sched_job
*s_job)
schedule_delayed_work(&next->work_tdr, sched->timeout);
}
}
+ spin_unlock_irqrestore(&sched->job_list_lock, flags);
}
static void amd_sched_job_begin(struct amd_sched_job *s_job)
{
struct amd_gpu_scheduler *sched = s_job->sched;
+ unsigned long flags;
+ spin_lock_irqsave(&sched->job_list_lock, flags);
+ list_add_tail(&s_job->node, &sched->ring_mirror_list);
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
list_first_entry_or_null(&sched->ring_mirror_list,
struct amd_sched_job, node) == s_job)
@@ -360,6 +368,7 @@ static void amd_sched_job_begin(struct amd_sched_job *s_job)
amd_sched_job_get(s_job);
schedule_delayed_work(&s_job->work_tdr, sched->timeout);
}
+ spin_unlock_irqrestore(&sched->job_list_lock, flags);
}
static void amd_sched_job_timedout(struct work_struct *work)
@@ -457,15 +466,10 @@ static void amd_sched_process_job(struct fence *f, struct
fence_cb *cb)
struct amd_sched_fence *s_fence =
container_of(cb, struct amd_sched_fence, cb);
struct amd_gpu_scheduler *sched = s_fence->sched;
- unsigned long flags;
atomic_dec(&sched->hw_rq_count);
- /* remove job from ring_mirror_list */
- spin_lock_irqsave(&sched->job_list_lock, flags);
- list_del_init(&s_fence->s_job->node);
amd_sched_job_finish(s_fence->s_job);
- spin_unlock_irqrestore(&sched->job_list_lock, flags);
amd_sched_fence_signal(s_fence);
@@ -478,7 +482,6 @@ static int amd_sched_main(void *param)
{
struct sched_param sparam = {.sched_priority = 1};
struct amd_gpu_scheduler *sched = (struct amd_gpu_scheduler *)param;
- unsigned long flags;
int r, count;
sched_setscheduler(current, SCHED_FIFO, &sparam);
@@ -503,10 +506,7 @@ static int amd_sched_main(void *param)
s_fence = sched_job->s_fence;
atomic_inc(&sched->hw_rq_count);
- spin_lock_irqsave(&sched->job_list_lock, flags);
- list_add_tail(&sched_job->node, &sched->ring_mirror_list);
amd_sched_job_begin(sched_job);
- spin_unlock_irqrestore(&sched->job_list_lock, flags);
fence = sched->ops->run_job(sched_job);
amd_sched_fence_scheduled(s_fence);
--
2.5.5
[PATCH 6/8] drm/amdgpu: properly abstract scheduler timeout handling
From: Christian König
The driver shouldn't mess with the scheduler internals.
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 15 ---
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 11 +--
drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 2 +-
5 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6c5ec32..ec42dd5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -759,7 +759,6 @@ void amdgpu_job_free_func(struct kref *refcount);
int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
struct amd_sched_entity *entity, void *owner,
struct fence **f);
-void amdgpu_job_timeout_func(struct work_struct *work);
struct amdgpu_ring {
struct amdgpu_device*adev;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 2bbeeb0..d8cac31 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -839,8 +839,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
p->job = NULL;
r = amd_sched_job_init(&job->base, &ring->sched,
- entity, amdgpu_job_timeout_func,
- amdgpu_job_free_func,
+ entity, amdgpu_job_free_func,
p->filp, &fence);
if (r) {
amdgpu_job_free(job);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index be4698b..32132f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -34,13 +34,14 @@ static void amdgpu_job_free_handler(struct work_struct *ws)
amd_sched_job_put(&job->base);
}
-void amdgpu_job_timeout_func(struct work_struct *work)
+static void amdgpu_job_timedout(struct amd_sched_job *s_job)
{
- struct amdgpu_job *job = container_of(work, struct amdgpu_job,
base.work_tdr.work);
+ struct amdgpu_job *job = container_of(s_job, struct amdgpu_job, base);
+
DRM_ERROR("ring %s timeout, last signaled seq=%u, last emitted
seq=%u\n",
- job->base.sched->name,
-
(uint32_t)atomic_read(&job->ring->fence_drv.last_seq),
- job->ring->fence_drv.sync_seq);
+ job->base.sched->name,
+ atomic_read(&job->ring->fence_drv.last_seq),
+ job->ring->fence_drv.sync_seq);
amd_sched_job_put(&job->base);
}
@@ -126,8 +127,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct
amdgpu_ring *ring,
return -EINVAL;
r = amd_sched_job_init(&job->base, &ring->sched,
- entity, amdgpu_job_timeout_func,
- amdgpu_job_free_func, owner, &fence);
+ entity, amdgpu_job_free_func, owner, &fence);
if (r)
return r;
@@ -197,4 +197,5 @@ err:
const struct amd_sched_backend_ops amdgpu_sched_ops = {
.dependency = amdgpu_job_dependency,
.run_job = amdgpu_job_run,
+ .timedout_job = amdgpu_job_timedout,
};
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index e8ee90f..f2ed8c5 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -362,6 +362,14 @@ static void amd_sched_job_begin(struct amd_sched_job
*s_job)
}
}
+static void amd_sched_job_timedout(struct work_struct *work)
+{
+ struct amd_sched_job *job = container_of(work, struct amd_sched_job,
+work_tdr.work);
+
+ job->sched->ops->timedout_job(job);
+}
+
/**
* Submit a job to the job queue
*
@@ -384,7 +392,6 @@ void amd_sched_entity_push_job(struct amd_sched_job
*sched_job)
int amd_sched_job_init(struct amd_sched_job *job,
struct amd_gpu_scheduler *sched,
struct amd_sched_entity *entity,
- void (*timeout_cb)(struct work_struct *work),
void (*free_cb)(struct kref *refcount),
void *owner, struct fence **fence)
{
@@ -397,7 +404,7 @@ int amd_sched_job_init(struct amd_sched_job *job,
return -ENOMEM;
job->s_fence->s_job = job;
- INIT_DELAYED_WORK(&job->work_tdr, timeout_cb);
+ INIT_DELAYED_WORK(&job->work_tdr, amd_sched_job_timedout);
job->free_callback = free_cb;
if (fence)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
b/drivers/gpu/dr
[PATCH 5/8] drm/amdgpu: remove use_shed hack in job cleanup
From: Christian König
Remembering the code path in a variable to cleanup
differently is usually not a good idea at all.
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 13 -
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 1 -
drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 1 -
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index f0fa485..be4698b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -86,7 +86,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev,
unsigned size,
return r;
}
-void amdgpu_job_free(struct amdgpu_job *job)
+static void amdgpu_job_free_resources(struct amdgpu_job *job)
{
struct fence *f;
unsigned i;
@@ -100,9 +100,6 @@ void amdgpu_job_free(struct amdgpu_job *job)
amdgpu_bo_unref(&job->uf_bo);
amdgpu_sync_free(&job->sync);
-
- if (!job->base.use_sched)
- kfree(job);
}
void amdgpu_job_free_func(struct kref *refcount)
@@ -111,6 +108,12 @@ void amdgpu_job_free_func(struct kref *refcount)
kfree(job);
}
+void amdgpu_job_free(struct amdgpu_job *job)
+{
+ amdgpu_job_free_resources(job);
+ kfree(job);
+}
+
int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
struct amd_sched_entity *entity, void *owner,
struct fence **f)
@@ -187,7 +190,7 @@ static struct fence *amdgpu_job_run(struct amd_sched_job
*sched_job)
err:
job->fence = fence;
- amdgpu_job_free(job);
+ amdgpu_job_free_resources(job);
return fence;
}
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index f3fd80b..e8ee90f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -373,7 +373,6 @@ void amd_sched_entity_push_job(struct amd_sched_job
*sched_job)
{
struct amd_sched_entity *entity = sched_job->s_entity;
- sched_job->use_sched = 1;
fence_add_callback(&sched_job->s_fence->base,
&sched_job->cb_free_job, amd_sched_free_job);
trace_amd_sched_job(sched_job);
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index ec55b9f..7e333fa 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -82,7 +82,6 @@ struct amd_sched_job {
struct amd_gpu_scheduler*sched;
struct amd_sched_entity *s_entity;
struct amd_sched_fence *s_fence;
- booluse_sched; /* true if the job goes to scheduler */
struct fence_cbcb_free_job;
struct work_struct work_free_job;
struct list_head node;
--
2.5.5
[PATCH 4/8] drm/amdgpu: fix coding style in amdgpu_job_free
From: Christian König
Ther should be a new line between code and decleration.
Also use amdgpu_ib_free() instead of releasing the member manually.
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 001030b..f0fa485 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -88,13 +88,14 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev,
unsigned size,
void amdgpu_job_free(struct amdgpu_job *job)
{
- unsigned i;
struct fence *f;
+ unsigned i;
+
/* use sched fence if available */
- f = (job->base.s_fence)? &job->base.s_fence->base : job->fence;
+ f = job->base.s_fence ? &job->base.s_fence->base : job->fence;
for (i = 0; i < job->num_ibs; ++i)
- amdgpu_sa_bo_free(job->adev, &job->ibs[i].sa_bo, f);
+ amdgpu_ib_free(job->adev, &job->ibs[i], f);
fence_put(job->fence);
amdgpu_bo_unref(&job->uf_bo);
--
2.5.5
[PATCH 3/8] drm/amdgpu: remove duplicated timeout callback
From: Christian König
No need for double housekeeping here.
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 5 +
drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 1 -
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 821bc89..f3fd80b 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -343,8 +343,6 @@ static void amd_sched_job_finish(struct amd_sched_job
*s_job)
struct amd_sched_job, node);
if (next) {
- INIT_DELAYED_WORK(&next->work_tdr,
- s_job->timeout_callback);
amd_sched_job_get(next);
schedule_delayed_work(&next->work_tdr, sched->timeout);
}
@@ -359,7 +357,6 @@ static void amd_sched_job_begin(struct amd_sched_job *s_job)
list_first_entry_or_null(&sched->ring_mirror_list,
struct amd_sched_job, node) == s_job)
{
- INIT_DELAYED_WORK(&s_job->work_tdr, s_job->timeout_callback);
amd_sched_job_get(s_job);
schedule_delayed_work(&s_job->work_tdr, sched->timeout);
}
@@ -401,7 +398,7 @@ int amd_sched_job_init(struct amd_sched_job *job,
return -ENOMEM;
job->s_fence->s_job = job;
- job->timeout_callback = timeout_cb;
+ INIT_DELAYED_WORK(&job->work_tdr, timeout_cb);
job->free_callback = free_cb;
if (fence)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 69840d7..ec55b9f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -87,7 +87,6 @@ struct amd_sched_job {
struct work_struct work_free_job;
struct list_head node;
struct delayed_work work_tdr;
- void (*timeout_callback) (struct work_struct *work);
void (*free_callback)(struct kref *refcount);
};
--
2.5.5
[PATCH 2/8] drm/amdgpu: remove begin_job/finish_job
From: Christian König
Completely pointless and confusing to use a callback
to call into the same code file.
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 --
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 13 +
drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 6 --
drivers/gpu/drm/amd/scheduler/sched_fence.c | 10 --
4 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index f0dafa5..001030b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -193,6 +193,4 @@ err:
const struct amd_sched_backend_ops amdgpu_sched_ops = {
.dependency = amdgpu_job_dependency,
.run_job = amdgpu_job_run,
- .begin_job = amd_sched_job_begin,
- .finish_job = amd_sched_job_finish,
};
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index f5ac01db2..821bc89 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -329,7 +329,7 @@ static void amd_sched_free_job(struct fence *f, struct
fence_cb *cb) {
/* job_finish is called after hw fence signaled, and
* the job had already been deleted from ring_mirror_list
*/
-void amd_sched_job_finish(struct amd_sched_job *s_job)
+static void amd_sched_job_finish(struct amd_sched_job *s_job)
{
struct amd_sched_job *next;
struct amd_gpu_scheduler *sched = s_job->sched;
@@ -351,7 +351,7 @@ void amd_sched_job_finish(struct amd_sched_job *s_job)
}
}
-void amd_sched_job_begin(struct amd_sched_job *s_job)
+static void amd_sched_job_begin(struct amd_sched_job *s_job)
{
struct amd_gpu_scheduler *sched = s_job->sched;
@@ -461,7 +461,7 @@ static void amd_sched_process_job(struct fence *f, struct
fence_cb *cb)
/* remove job from ring_mirror_list */
spin_lock_irqsave(&sched->job_list_lock, flags);
list_del_init(&s_fence->s_job->node);
- sched->ops->finish_job(s_fence->s_job);
+ amd_sched_job_finish(s_fence->s_job);
spin_unlock_irqrestore(&sched->job_list_lock, flags);
amd_sched_fence_signal(s_fence);
@@ -475,6 +475,7 @@ static int amd_sched_main(void *param)
{
struct sched_param sparam = {.sched_priority = 1};
struct amd_gpu_scheduler *sched = (struct amd_gpu_scheduler *)param;
+ unsigned long flags;
int r, count;
sched_setscheduler(current, SCHED_FIFO, &sparam);
@@ -499,7 +500,11 @@ static int amd_sched_main(void *param)
s_fence = sched_job->s_fence;
atomic_inc(&sched->hw_rq_count);
- amd_sched_job_pre_schedule(sched, sched_job);
+ spin_lock_irqsave(&sched->job_list_lock, flags);
+ list_add_tail(&sched_job->node, &sched->ring_mirror_list);
+ amd_sched_job_begin(sched_job);
+ spin_unlock_irqrestore(&sched->job_list_lock, flags);
+
fence = sched->ops->run_job(sched_job);
amd_sched_fence_scheduled(s_fence);
if (fence) {
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 690ae4b..69840d7 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -110,8 +110,6 @@ static inline struct amd_sched_fence
*to_amd_sched_fence(struct fence *f)
struct amd_sched_backend_ops {
struct fence *(*dependency)(struct amd_sched_job *sched_job);
struct fence *(*run_job)(struct amd_sched_job *sched_job);
- void (*begin_job)(struct amd_sched_job *sched_job);
- void (*finish_job)(struct amd_sched_job *sched_job);
};
enum amd_sched_priority {
@@ -160,10 +158,6 @@ int amd_sched_job_init(struct amd_sched_job *job,
void (*timeout_cb)(struct work_struct *work),
void (*free_cb)(struct kref* refcount),
void *owner, struct fence **fence);
-void amd_sched_job_pre_schedule(struct amd_gpu_scheduler *sched ,
- struct amd_sched_job *s_job);
-void amd_sched_job_finish(struct amd_sched_job *s_job);
-void amd_sched_job_begin(struct amd_sched_job *s_job);
static inline void amd_sched_job_get(struct amd_sched_job *job)
{
if (job)
diff --git a/drivers/gpu/drm/amd/scheduler/sched_fence.c
b/drivers/gpu/drm/amd/scheduler/sched_fence.c
index 6bdc9b7..71931bc 100644
--- a/drivers/gpu/drm/amd/scheduler/sched_fence.c
+++ b/drivers/gpu/drm/amd/scheduler/sched_fence.c
@@ -58,16 +58,6 @@ void amd_sched_fence_signal(struct amd_sched_fence *fence)
FENCE_TRACE(&fence->base, "was already signaled\n");
}
-void amd_sched_job_pre_schedule(struct amd_gpu_scheduler *sched ,
- str
[PATCH 1/8] drm/amdgpu: fix coding style in the scheduler v2
From: Christian König
v2: fix even more
Signed-off-by: Christian König
Reviewed-by: Monk.Liu
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 25 +++--
drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 21 -
drivers/gpu/drm/amd/scheduler/sched_fence.c | 9 +
3 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index c16248c..f5ac01db2 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -320,7 +320,9 @@ static bool amd_sched_entity_in(struct amd_sched_job
*sched_job)
}
static void amd_sched_free_job(struct fence *f, struct fence_cb *cb) {
- struct amd_sched_job *job = container_of(cb, struct amd_sched_job,
cb_free_job);
+ struct amd_sched_job *job = container_of(cb, struct amd_sched_job,
+cb_free_job);
+
schedule_work(&job->work_free_job);
}
@@ -341,7 +343,8 @@ void amd_sched_job_finish(struct amd_sched_job *s_job)
struct amd_sched_job, node);
if (next) {
- INIT_DELAYED_WORK(&next->work_tdr,
s_job->timeout_callback);
+ INIT_DELAYED_WORK(&next->work_tdr,
+ s_job->timeout_callback);
amd_sched_job_get(next);
schedule_delayed_work(&next->work_tdr, sched->timeout);
}
@@ -353,7 +356,8 @@ void amd_sched_job_begin(struct amd_sched_job *s_job)
struct amd_gpu_scheduler *sched = s_job->sched;
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
- list_first_entry_or_null(&sched->ring_mirror_list, struct
amd_sched_job, node) == s_job)
+ list_first_entry_or_null(&sched->ring_mirror_list,
+struct amd_sched_job, node) == s_job)
{
INIT_DELAYED_WORK(&s_job->work_tdr, s_job->timeout_callback);
amd_sched_job_get(s_job);
@@ -374,7 +378,7 @@ void amd_sched_entity_push_job(struct amd_sched_job
*sched_job)
sched_job->use_sched = 1;
fence_add_callback(&sched_job->s_fence->base,
- &sched_job->cb_free_job,
amd_sched_free_job);
+ &sched_job->cb_free_job, amd_sched_free_job);
trace_amd_sched_job(sched_job);
wait_event(entity->sched->job_scheduled,
amd_sched_entity_in(sched_job));
@@ -382,11 +386,11 @@ void amd_sched_entity_push_job(struct amd_sched_job
*sched_job)
/* init a sched_job with basic field */
int amd_sched_job_init(struct amd_sched_job *job,
- struct amd_gpu_scheduler *sched,
- struct amd_sched_entity *entity,
- void (*timeout_cb)(struct
work_struct *work),
- void (*free_cb)(struct kref
*refcount),
- void *owner, struct fence
**fence)
+ struct amd_gpu_scheduler *sched,
+ struct amd_sched_entity *entity,
+ void (*timeout_cb)(struct work_struct *work),
+ void (*free_cb)(struct kref *refcount),
+ void *owner, struct fence **fence)
{
INIT_LIST_HEAD(&job->node);
kref_init(&job->refcount);
@@ -504,7 +508,8 @@ static int amd_sched_main(void *param)
if (r == -ENOENT)
amd_sched_process_job(fence, &s_fence->cb);
else if (r)
- DRM_ERROR("fence add callback failed (%d)\n",
r);
+ DRM_ERROR("fence add callback failed (%d)\n",
+ r);
fence_put(fence);
} else {
DRM_ERROR("Failed to run job!\n");
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 070095a..690ae4b 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -94,7 +94,8 @@ struct amd_sched_job {
extern const struct fence_ops amd_sched_fence_ops;
static inline struct amd_sched_fence *to_amd_sched_fence(struct fence *f)
{
- struct amd_sched_fence *__f = container_of(f, struct amd_sched_fence,
base);
+ struct amd_sched_fence *__f = container_of(f, struct amd_sched_fence,
+ base);
if (__f->base.ops == &amd_sched_fence_ops)
return __f;
@@ -154,21 +155,23 @@ struct amd_sched_fence *amd_sched_fence_create(
void amd_sched_fence_s
[PATCH 0/8] scheduler code cleanup
Just some general cleanup in the GPU scheduler. Christian König (8): drm/amdgpu: fix coding style in the scheduler v2 drm/amdgpu: remove begin_job/finish_job drm/amdgpu: remove duplicated timeout callback drm/amdgpu: fix coding style in amdgpu_job_free drm/amdgpu: remove use_shed hack in job cleanup drm/amdgpu: properly abstract scheduler timeout handling drm/amdgpu: move locking into the functions who need it drm/amdgpu: fix and cleanup job destruction drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 - drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 49 +++ drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 89 ++- drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 42 - drivers/gpu/drm/amd/scheduler/sched_fence.c | 19 ++ 6 files changed, 86 insertions(+), 119 deletions(-) -- 2.5.5
[pull] amdgpu drm-next-4.7
Hi Dave, A few fixes and cleanups for amdgpu for 4.7 The following changes since commit 3b59c344ab6e2d00b0f4ad946024572618c87502: drm/amd/powerplay: fix bugs of checking if dpm is running on Tonga (2016-05-18 09:22:30 -0400) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-next-4.7 for you to fetch changes up to 425c452b547622fb728c9ed4fe99c3f2f51d816d: drm/amd: add Kconfig dependency for ACP on DRM_AMDGPU (2016-05-25 09:44:15 -0400) Jeff Mahoney (1): drm/amd: add Kconfig dependency for ACP on DRM_AMDGPU Mario Kleiner (1): drm/amdgpu: Fix hdmi deep color support. Monk Liu (1): drm/amdgpu: fix bug in fence driver fini Muhammad Falak R Wani (3): drm/amd/powerplay/hwmgr: use kmemdup drm/amd/powerplay/hwmgr: use kmemdup drm/amd/powerplay/hwmgr: use kmemdup drivers/gpu/drm/amd/acp/Kconfig | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c| 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 2 +- drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 6 +++--- drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 6 +++--- drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 6 +++--- 6 files changed, 16 insertions(+), 15 deletions(-)
[PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM
On Wed, May 25, 2016 at 12:53:01AM +0200, Peter Wu wrote:
> Since "PCI: Add runtime PM support for PCIe ports", the parent PCIe port
> can be runtime-suspended which disables power resources via ACPI. This
> is incompatible with DSM, resulting in a GPU device which is still in D3
> and locks up the kernel on resume.
>
> Mirror the behavior of Windows 8 and newer[1] (as observed via an AMLi
> debugger trace) and stop using the DSM functions for D3cold when power
> resources are available on the parent PCIe port.
>
> [1]:
> https://msdn.microsoft.com/windows/hardware/drivers/bringup/firmware-requirements-for-d3cold
>
> Signed-off-by: Peter Wu
> ---
> drivers/gpu/drm/nouveau/nouveau_acpi.c | 34
> ++
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> index df9f73e..e469df7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> @@ -46,6 +46,7 @@ static struct nouveau_dsm_priv {
> bool dsm_detected;
> bool optimus_detected;
> bool optimus_flags_detected;
> + bool optimus_skip_dsm;
> acpi_handle dhandle;
> acpi_handle rom_handle;
> } nouveau_dsm_priv;
> @@ -212,8 +213,26 @@ static const struct vga_switcheroo_handler
> nouveau_dsm_handler = {
> .get_client_id = nouveau_dsm_get_client_id,
> };
>
> +/* Firmware supporting Windows 8 or later do not use _DSM to put the device
> into
> + * D3cold, they instead rely on disabling power resources on the parent. */
> +static bool nouveau_pr3_present(struct pci_dev *pdev)
> +{
> + struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> + struct acpi_device *ad;
Nit: please call this adev instead of ad.
> +
> + if (!parent_pdev)
> + return false;
> +
> + ad = ACPI_COMPANION(&parent_pdev->dev);
> + if (!ad)
> + return false;
> +
> + return ad->power.flags.power_resources;
Is this sufficient to tell if the parent device has _PR3? I thought it
returns true if it has power resources in general, not necessarily _PR3.
Otherwise this looks okay to me.
objtool warning: "duplicate frame pointer save"
On Wed, May 25, 2016 at 10:56 AM, Josh Poimboeuf wrote: > > I used to have a STACKTOOL_IGNORE_INSN macro that would tell the tool to > skip warnings for specific instructions in inline asm: > > Here's an example of it how it was used: Ok, looking at that, I'm starting to suspect that it is simpler to just use STACK_FRAME_NON_STANDARD and mark the two functions that use this particular inline asm with the odd %rbp problem. It's a rather special case, after all. Linus
[PATCH 06/14] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers
On Tue, May 24, 2016 at 10:23 PM, Philipp Zabel
wrote:
> Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying:
>> Use the drm_plane_helper_update/disable() and drm_helper_crtc_mode_set()
>> transistional atomic helpers. The crtc->mode_set_nofb callback is added
>> so that the primary plane is no longer tied to the CRTC. Check/update
>> logics are separated to make sure crtc->mode_set_nofb and
>> plane->atomic_update
>> are always successful. Also, some necessary logics are tweaked for a smooth
>> transition.
>>
>> Signed-off-by: Liu Ying
>> ---
>> drivers/gpu/drm/imx/ipuv3-crtc.c | 173 +++--
>> drivers/gpu/drm/imx/ipuv3-plane.c | 516
>> +++---
>> drivers/gpu/drm/imx/ipuv3-plane.h | 14 +-
>> drivers/gpu/ipu-v3/ipu-dc.c | 5 +-
>> drivers/gpu/ipu-v3/ipu-di.c | 3 -
>> 5 files changed, 400 insertions(+), 311 deletions(-)
>>
> [...]
>> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
>> b/drivers/gpu/drm/imx/ipuv3-plane.c
>> index 30cedbb..4511b84 100644
>> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
>> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
>> @@ -16,6 +16,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "video/imx-ipu-v3.h"
>> #include "ipuv3-plane.h"
>> @@ -51,12 +52,15 @@ int ipu_plane_irq(struct ipu_plane *ipu_plane)
>>IPU_IRQ_EOF);
>> }
>>
>> -int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer
>> *fb,
>> -int x, int y)
>> +int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer
>> *fb)
>> {
>> - struct drm_gem_cma_object *cma_obj[3];
>> - unsigned long eba, ubo, vbo;
>> + struct drm_gem_cma_object *cma_obj[3], *old_cma_obj[3];
>> + struct drm_plane_state *state = ipu_plane->base.state;
>> + struct drm_framebuffer *old_fb = state->fb;
>> + unsigned long eba, ubo, vbo, old_eba, old_ubo, old_vbo;
>
> It looks to me like eba, ubo, vbo are good candidates to be put into a
> custom ipu_plane_state structure during check.
Yes for the first impression. I'll think about that more.
If the required change is too invasive and you accept, I'd like to
customize ipu_plane_state with another patch(set) after this one
is landed.
Regards,
Liu Ying
>
> regards
> Philipp
>
DRM DMA Engine
Hi all, Currently I am trying to develop a DRM driver that will use Xilinx VDMA to transfer video data to a HDMI TX Phy and I am facing a difficulty regarding the understanding of the DRM DMA Engine. I looked at several sources and at the DRM core source but the flow of creating and interfacing with the DMA controller is still not clear to me. At DRI web page the X server is mentioned. Does it mean that the channel creation and handling is done by the X server? If so, what is the DRM driver responsible to do then and what exactly does the DRM core do? As I am using Xilinx VDMA do you foresee any special implementation details? Just for reference here is the description of the Xilinx VDMA: "The Advanced eXtensible Interface Video Direct Memory Access (AXI VDMA) core is a soft Xilinx Intellectual Property (IP) core providing high-bandwidth direct memory access between memory and AXI4-Stream video type target peripherals including peripherals which support AXI4-Stream Video Protocol." The driver is available at "drivers/dma/xilinx/xilinx_vdma.c". Another important point: I am using PCI Express connected to a FPGA which has all the necessary components (Xilinx VDMA, I2S, ...) and the HDMI TX Phy. Looking forward to you help. Best regards, Jose Miguel Abreu
[PATCH] drm/dp/mst: Always clear proposed vcpi table for port.
Not clearing mst manager's proposed vcpis table for
destroyed connectors when the manager is stopped
leaves it pointing to unrefernced memory,
this causes pagefault when the manager is
restarted when plugging back a branch.
Fixes: 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")
Signed-off-by: Andrey Grodzovsky
Reviewed-by: Lyude
Cc: stable at vger.kernel.org
Cc: Mykola Lysenko
Cc: Alex Deucher
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9971c46..cd6014b 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2881,11 +2881,9 @@ static void drm_dp_destroy_connector_work(struct
work_struct *work)
drm_dp_port_teardown_pdt(port, port->pdt);
if (!port->input && port->vcpi.vcpi > 0) {
- if (mgr->mst_state) {
- drm_dp_mst_reset_vcpi_slots(mgr, port);
- drm_dp_update_payload_part1(mgr);
- drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
- }
+ drm_dp_mst_reset_vcpi_slots(mgr, port);
+ drm_dp_update_payload_part1(mgr);
+ drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
}
kref_put(&port->kref, drm_dp_free_mst_port);
--
1.9.1
[PATCH 07/14] drm/imx: atomic phase 2 step 1: Wire up state ->reset, ->duplicate and ->destroy
On Tue, May 24, 2016 at 10:23 PM, Philipp Zabel
wrote:
> Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying:
>> Wire up CRTCs', planes' and connectors' ->reset, ->duplicate and ->destroy
>> state
>> hooks to use the default implementations from the atomic helper library.
>> The helpers track each DRM object state.
>>
>> Signed-off-by: Liu Ying
>> ---
>> drivers/gpu/drm/bridge/dw-hdmi.c | 19 +++
>> drivers/gpu/drm/imx/imx-drm-core.c | 2 ++
>> drivers/gpu/drm/imx/imx-ldb.c | 4
>> drivers/gpu/drm/imx/imx-tve.c | 4
>> drivers/gpu/drm/imx/ipuv3-crtc.c | 4
>> drivers/gpu/drm/imx/ipuv3-plane.c | 4
>> drivers/gpu/drm/imx/parallel-display.c | 4
>> 7 files changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
>> b/drivers/gpu/drm/bridge/dw-hdmi.c
>> index 9795b72..6541b3c 100644
>> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
>> @@ -1508,14 +1508,6 @@ static void dw_hdmi_connector_force(struct
>> drm_connector *connector)
>> mutex_unlock(&hdmi->mutex);
>> }
>>
>> -static const struct drm_connector_funcs dw_hdmi_connector_funcs = {
>> - .dpms = drm_helper_connector_dpms,
>> - .fill_modes = drm_helper_probe_single_connector_modes,
>> - .detect = dw_hdmi_connector_detect,
>> - .destroy = dw_hdmi_connector_destroy,
>> - .force = dw_hdmi_connector_force,
>> -};
>> -
>> static const struct drm_connector_funcs dw_hdmi_atomic_connector_funcs = {
>
> Might as well remove the _atomic_ form the structure's name.
Good point. Will do.
Regards,
Liu Ying
>
> regards
> Philipp
>
[Bug 96213] Missing DRI support for TRIDENT Cyberblade-i1
https://bugs.freedesktop.org/show_bug.cgi?id=96213 Xavier Bachelot changed: What|Removed |Added CC||xavier at bachelot.org Component|DRM/Via |DRM/other Assignee|openchrome-devel at lists.free |dri-devel at lists.freedesktop |desktop.org |.org --- Comment #1 from Xavier Bachelot --- The VT8361 is indeed a VIA chipset, but the integrated graphic chipset is from Trident. Re-assigning to DRM/Others, I don't know which DRM module is supposed to drive this, if any. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/69ca1450/attachment.html>
[RFC] drm: Introduce max width and height properties for planes
On 05/25/2016 12:40 PM, Daniel Vetter wrote: > On Wed, May 25, 2016 at 12:03:39PM +0530, Archit Taneja wrote: >> High resoultion modes (4K/5K) are supported by encoder/crtc hardware, but >> the backing hardware planes to scanout these buffers generally don't >> support such large widths. Two (or more) hardware planes are used in >> conjunction to scan out the entire width. > > "generally" seems to be a bit an overstatement. Most sensible hw has no > problem at all scanning that kind of stuff out ;-) > >> One way to support this is to virtualize the drm_plane objects that we >> expose to userspace, and let the kms driver internally figure out whether >> it needs to club multiple hardware pipes to run a particular mode. >> >> The other option is to represent a drm_plane as it is in hardware, and >> leave it to userspace use to manage multiple drm_planes for scanning >> out these modes. >> >> The advantage of the latter option is that the kms driver doesn't get >> too complicated. It doesn't need to do the decision making on how to >> distibute the hardware resources among the drm_planes. It also allows >> userspace to continue doing some of the heavy lifting (figure out >> the most power efficient combination of pipes, estimate bandwidth >> consumption etc) which would have to be moved to the kernel if we >> virtualized planes. >> >> In order to achieve the latter, a drm_plane should expose an immutable >> property to userspace describing the maximum width and height it can >> support. Userspace can then prepare the drm_planes accordingly and >> commit the state. This patch introduces these properties. >> >> The main disadvantage is that legacy userspace (i.e. calling SetCrtc >> on a high resolution framebuffer) wouldn't work. This, on the other >> hand, wouldn't be a problem if we virtualized the planes and manipulated >> the hardware pipes in the kenrel. One way around this is to fail large >> resolution modes when using legacy setcrtc, or introduce some minimal >> helpers in the kernel that automatically use multiple drm_planes when >> we try to set such a mode. A solution to this issue isn't a part of the >> RFC yet. >> >> We're looking for feedback here and wondering whether this is the right >> way to go or not. > > So just this week there was a bit a discussion on irc (and I haven't > gotten around to typing the writeup and asking everyone for feedback on > it) on new properties. Super-short summary: > > - new properties are new abi like anything else > > - drm wants a userspace demonstration vehicle to prove new abi which is: >open source, patches reviewed & tested by respective canonical upstream >(so not a vendor fork or something like that). Testcases are explicitly >not enough, it needs to be the real deal. > > - for props specifically which are meant to be used/interpreted by >compositor this means a patch for drm_hwcomposer, one of the many >wayland compositors there are (weston, ozone, mutter), or a patch for >xfree86-video-modesetting. Other userspace thingies are probably a bit >too much fringe to count. > > - this isn't really new, but thus far arm simply sailed in the shadow of >intel doing all that hard work. Now some arm drivers start to pull >ahead, adding new ABI that's not yet proven by i915 folks, and with >success comes a bit more responsibility. That sounds fair enough. > > For the interface itself just a few questions: > - We should make the cursor size stuff obselete by this, and instead first >look up the size limits of the cursor plane, before checking those >legacy limits. Yeah, I guess querying the DRM_CAP_CURSOR_WIDTH/HEIGHT caps would become obsolete. > - Is the size/width really independent of e.g. rotation/pixel format/... >Should it be the maximum that's possible under best circumstance (things >could still fail), or the minimum that's guaranteed to work everwhere. >This is the kind of stuff we need the userspace part for, too. Yeah, it isn't independent of these parameters. I'm not entirely sure about this either. Does it make sense to impose a rule that the user first sets the rotation/format plane properties, and only then read the maximum width? I'm assuming user space hates such kind of stuff. If we use the 'best circumstance' max_width, we can first start with a minimum number of planes that need to be grouped to achieve the target mode. If that fails the atomic test, then we can try to add one plane at a time, and reduce the width for each plane. If we use the minimum/'guaranteed to work' max_width, we'll get a higher number of planes than needed for this mode. This would pass the atomic test. We could then reduce a plane at a time and see when we fail the atomic test. I guess we need to chose the one that's more probable to get right the first time. Considering only pixel formats for now, the minimum/'guaranteed to work' would map to the RGB formats. The best circumstance ones would probably b
[Nouveau] [PATCH 9/9] drm: Turn off crtc before tearing down its data structure
On Wed, May 25, 2016 at 12:51 PM, Lukas Wunner wrote:
>
> On Tue, May 24, 2016 at 11:30:42PM +0200, Daniel Vetter wrote:
>> On Tue, May 24, 2016 at 06:03:27PM +0200, Lukas Wunner wrote:
>> > When a drm_crtc structure is destroyed with drm_crtc_cleanup(), the DRM
>> > core does not turn off the crtc first and neither do the drivers. With
>> > nouveau, radeon and amdgpu, this causes a runtime pm ref to be leaked on
>> > driver unload if at least one crtc was enabled.
>> >
>> > (See usage of have_disp_power_ref in nouveau_crtc_set_config(),
>> > radeon_crtc_set_config() and amdgpu_crtc_set_config()).
>> >
>> > Fixes: 5addcf0a5f0f ("nouveau: add runtime PM support (v0.9)")
>> > Cc: Dave Airlie
>> > Tested-by: Karol Herbst
>> > Signed-off-by: Lukas Wunner
>>
>> This is a core regression, we fixed it again. Previously when unreference
>> drm_planes the core made sure that it's not longer in use, which had the
>> side effect of shutting everything off in module unload.
>>
>> For a bunch of reasons we've stopped doing that, but that turned out to be
>> a mistake. It's fixed since
>>
>> commit f2d580b9a8149735cbc4b59c4a8df60173658140
>> Author: Maarten Lankhorst
>> Date: Wed May 4 14:38:26 2016 +0200
>>
>> drm/core: Do not preserve framebuffer on rmfb, v4.
>>
>> Your patch shouldn't be needed with that any more. If it still is it's
>> most likely the fbdev cleanup done too late, but you /should/ get a big
>> WARNING splat in that case from drm_mode_config_cleanup().
>
> I tested it and at least with nouveau, the above-mentioned commit does *not*
> solve the issue, so patch [9/9] of this series is still needed. I do not get
> a WARN splat when unloading nouveau.
With legacy kms the only way to keep a crtc enabled is to display a
drm_framebuffer on it. And drm_mode_config_cleanup has a WARN_ON if
framebuffers are left behind. There's a bunch of options:
- nouveau somehow manages to keep the crtc on without a framebuffer
- nouveau somehow leaks a drm_framebuffer, but removes it from the fb_list
- something else
There's still no need to forcefully shut down crtc at cleanup time in
the core, this is still a driver bug. So yes your patch might be
needed, but it's not the right fix.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
DRM DMA Engine
Hi all, Currently I am trying to develop a DRM driver that will use Xilinx VDMA to transfer video data to a HDMI TX Phy and I am facing a difficulty regarding the understanding of the DRM DMA Engine. I looked at several sources and at the DRM core source but the flow of creating and interfacing with the DMA controller is still not clear to me. At DRI web page the X server is mentioned. Does it mean that the channel creation and handling is done by the X server? If so, what is the DRM driver responsible to do then and what exactly does the DRM core do? As I am using Xilinx VDMA do you foresee any special implementation details? Just for reference here is the description of the Xilinx VDMA: "The Advanced eXtensible Interface Video Direct Memory Access (AXI VDMA) core is a soft Xilinx Intellectual Property (IP) core providing high-bandwidth direct memory access between memory and AXI4-Stream video type target peripherals including peripherals which support AXI4-Stream Video Protocol." The driver is available at "drivers/dma/xilinx/xilinx_vdma.c". Another important point: I am using PCI Express connected to a FPGA which has all the necessary components (Xilinx VDMA, I2S, ...) and the HDMI TX Phy. Looking forward to you help. Best regards, Jose Miguel Abreu
[PATCHv3 1/1] drm/imx: convey the pixelclk-active and de-active flags from DT to the ipu-di driver
The 'de-active' and 'pixelclk-active' DT properties are evaluated
by of_parse_display_timing() called from of_get_drm_display_mode(),
but later lost in the conversion from videomode.flags to
drm_display_mode.flags.
Use an open coded version of of_get_drm_display_mode() to get access
to these flags and make sure they are passed on to the ipu-di driver.
Changes vs. v2:
- removed patches which have already been applied
- created a drm_bus_flags_from_videomode() helper to prevent code
duplication as suggested by Philipp Zabel
Changes vs. v1:
- rebased on top of https://patchwork.kernel.org/patch/9113791/ as
per Philipp Zabel's request
GIT: [PATCHv2 1/3] drm/imx: imx-ldb: honor 'native-mode' property when
GIT: [PATCHv2 2/3] drm/imx: convey the pixelclk-active and de-active flags
GIT: [PATCHv2 3/3] drm/imx: remove dead code
Signed-off-by: Lothar WaÃmann
---
drivers/gpu/drm/drm_modes.c| 20 +++-
drivers/gpu/drm/imx/imx-ldb.c | 15 ++-
drivers/gpu/drm/imx/parallel-display.c | 16 +---
include/drm/drm_modes.h| 5 +++--
4 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 7def3d5..7c97d5b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -655,6 +655,21 @@ void drm_display_mode_to_videomode(const struct
drm_display_mode *dmode,
}
EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
+void drm_bus_flags_from_videomode(const struct videomode *vm, u32 *bus_flags)
+{
+ *bus_flags = 0;
+ if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+ *bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
+ if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+ *bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+
+ if (vm->flags & DISPLAY_FLAGS_DE_LOW)
+ *bus_flags |= DRM_BUS_FLAG_DE_LOW;
+ if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+ *bus_flags |= DRM_BUS_FLAG_DE_HIGH;
+}
+EXPORT_SYMBOL_GPL(drm_bus_flags_from_videomode);
+
#ifdef CONFIG_OF
/**
* of_get_drm_display_mode - get a drm_display_mode from devicetree
@@ -670,7 +685,8 @@ EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
* 0 on success, a negative errno code when no of videomode node was found.
*/
int of_get_drm_display_mode(struct device_node *np,
- struct drm_display_mode *dmode, int index)
+ struct drm_display_mode *dmode, u32 *bus_flags,
+ int index)
{
struct videomode vm;
int ret;
@@ -680,6 +696,8 @@ int of_get_drm_display_mode(struct device_node *np,
return ret;
drm_display_mode_from_videomode(&vm, dmode);
+ if (bus_flags)
+ drm_bus_flags_from_videomode(&vm, bus_flags);
pr_debug("%s: got %dx%d display mode from %s\n",
of_node_full_name(np), vm.hactive, vm.vactive, np->name);
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index b2dc4df..bb9d745 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -65,7 +65,8 @@ struct imx_ldb_channel {
int edid_len;
struct drm_display_mode mode;
int mode_valid;
- int bus_format;
+ u32 bus_format;
+ u32 bus_flags;
};
struct bus_mux {
@@ -102,8 +103,10 @@ static int imx_ldb_connector_get_modes(struct
drm_connector *connector)
struct drm_display_info *di = &connector->display_info;
num_modes =
imx_ldb_ch->panel->funcs->get_modes(imx_ldb_ch->panel);
- if (!imx_ldb_ch->bus_format && di->num_bus_formats)
+ if (!imx_ldb_ch->bus_format && di->num_bus_formats) {
imx_ldb_ch->bus_format = di->bus_formats[0];
+ imx_ldb_ch->bus_flags = di->bus_flags;
+ }
if (num_modes > 0)
return num_modes;
}
@@ -202,7 +205,8 @@ static void imx_ldb_encoder_prepare(struct drm_encoder
*encoder)
break;
}
- imx_drm_set_bus_format(encoder, bus_format);
+ imx_drm_set_bus_config(encoder, bus_format, 2, 3,
+ imx_ldb_ch->bus_flags);
}
static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
@@ -558,7 +562,7 @@ static int imx_ldb_bind(struct device *dev, struct device
*master, void *data)
ret = of_property_read_u32(child, "reg", &i);
if (ret || i < 0 || i > 1)
- return -EINVAL;
+ return ret ?: -EINVAL;
if (dual && i > 0) {
dev_warn(dev, "dual-channel mode, ignoring second
output\n");
@@ -602,7 +606,8 @@ static int imx_ldb_bind(struct device *dev, struct device
*master, void *data)
GFP_KERNEL);
} else if (!channel->panel) {
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote:
> On 05/25/2016 12:40 PM, Daniel Vetter wrote:
> >- Is the size/width really independent of e.g. rotation/pixel format/...
> > Should it be the maximum that's possible under best circumstance (things
> > could still fail), or the minimum that's guaranteed to work everwhere.
> > This is the kind of stuff we need the userspace part for, too.
>
> Yeah, it isn't independent of these parameters. I'm not entirely sure
> about this either.
>
> Does it make sense to impose a rule that the user first sets the
> rotation/format plane properties, and only then read the maximum
> width? I'm assuming user space hates such kind of stuff.
>
> If we use the 'best circumstance' max_width, we can first start
> with a minimum number of planes that need to be grouped to achieve
> the target mode. If that fails the atomic test, then we can try to
> add one plane at a time, and reduce the width for each plane.
>
> If we use the minimum/'guaranteed to work' max_width, we'll get
> a higher number of planes than needed for this mode. This would pass
> the atomic test. We could then reduce a plane at a time and see when
> we fail the atomic test.
>
> I guess we need to chose the one that's more probable to get right
> the first time. Considering only pixel formats for now, the
> minimum/'guaranteed to work' would map to the RGB formats. The best
> circumstance ones would probably be the planar video formats. Since
> we use RGB more often, the minimum one might make more sense.
>
> We could, of course, give the property a range of max widths to
> confuse user space even more.
An entirely different idea for cases where a simple hint property doesn't
work (other ideas floating around are can_scale, to give a hint whether a
plan can at least in theory up/downscale, or not at all), is that the
kernel gives more specific hints about what it would like to change.
So if userspace asks for a plane, but for the given pixel format it's too
wide, the kernel could return a new proposed value for width. That would
be super-flexible and could cover all kinds of use-case like rotation
needing a specific tiling (fb_modifier) or specific pixel format, or
specific stride.
For the case at hand there's even more worms: What about stride
requirements? Afaik on some hw you just need to split the buffers into 2
planes, but can keep the wide stride (since the limit is the size of the
linebuffers in the hw). On others you need to split the buffer itself into
2, because the plane hw can't cope with huge strides. Again might depend
upon the pixel format.
So in a way height/width is both too much information and not precise
enough. Entirely different approches:
- We just add a might_need_split_plane prop to crtcs where this might be
needed. Userspace then gets to retry with split buffers if it doesn't
work with a huge one.
- When I discussed this with qualcom folks for msm we concluded that the
simplest approach would be to hide this in the kernel. So if you have a
too wide plane, and need 2 hw planes to scan it out, then do that
transparently in the kernel. Of course this means that there will be 1
(or 3 if you need a 2x2 split) fewer planes available, but userspace
needs to iteratively build up the plane config using ATOMIC_TEST anyway.
If possible for your hw I'm heavily leaning towards this last approach. If
fits entirely into the current atomic design, and all the complexity is
restricted to your driver (you need to have some allocation map between
drm planes and real hw planes, but that's it).
Thoughts?
-Daniel
>
> Archit
>
> >
> >Cheers, Daniel
> >
> >>Signed-off-by: Archit Taneja
> >>---
> >> drivers/gpu/drm/drm_atomic.c | 9 +
> >> drivers/gpu/drm/drm_crtc.c | 38 ++
> >> include/drm/drm_crtc.h | 6 ++
> >> 3 files changed, 53 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >>index 8ee1db8..fded22a 100644
> >>--- a/drivers/gpu/drm/drm_atomic.c
> >>+++ b/drivers/gpu/drm/drm_atomic.c
> >>@@ -839,6 +839,15 @@ static int drm_atomic_plane_check(struct drm_plane
> >>*plane,
> >>return -ERANGE;
> >>}
> >>
> >>+ if (plane->max_width && (state->src_w >> 16) > plane->max_width ||
> >>+ plane->max_height && (state->src_h >> 16) > plane->max_height) {
> >>+ DRM_DEBUG_ATOMIC("Invalid source width/height "
> >>+"%u.%06ux%u.%06u\n",
> >>+state->src_w >> 16, ((state->src_w & 0x) *
> >>15625) >> 10,
> >>+state->src_h >> 16, ((state->src_h & 0x) *
> >>15625) >> 10);
> >>+ return -ERANGE;
> >>+ }
> >>+
> >>fb_width = state->fb->width << 16;
> >>fb_height = state->fb->height << 16;
> >>
> >>diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> >>index e08f962..f2d3b92 100644
> >>--- a/drivers/gpu/drm/d
[PATCH 1/2] drm/exynos: g2d: Add support for old S5Pv210 type
Hey Krzysztof, Krzysztof Kozlowski wrote: > On 05/24/2016 06:05 PM, Tobias Jakobi wrote: >> Hello Krzysztof, >> >> >> Krzysztof Kozlowski wrote: >>> On 05/24/2016 03:49 PM, Tobias Jakobi wrote: Hello Krzysztof, are you sure that these are the only differences. Because AFAIK there are quite a few more: - DMA submission of commands - blend mode / rounding - solid fill - YCrCb support - and probably more One would need to add least split the command list parser into a v3 and v41 version to accomodate for the differences. In fact userspace/libdrm would need to know which hw type it currently uses, but you currently always return 4.1 in the corresponding ioctl. >>> >>> Eh, so probably my patch does not cover fully the support for v3 G2D. I >>> looked mostly at the differences between v3 and v4 in the s5p-g2d driver >>> itself. However you are right that this might be not sufficient because >>> exynos-g2d moved forward and is different than s5p-g2d. >>> Krzysztof Kozlowski wrote: > The non-DRM s5p-g2d driver supports two versions of G2D: v3.0 on > S5Pv210 and v4.x on Exynos 4x12 (or newer). The driver for 3.0 device > version is doing two things differently: > 1. Before starting the render process, it invalidates caches (pattern, >source buffer and mask buffer). Cache control is not present on v4.x >device. > 2. Scalling is done through StretchEn command (in BITBLT_COMMAND_REG >register) instead of SRC_SCALE_CTRL_REG as in v4.x. However the >exynos_drm_g2d driver does not implement the scalling so this >difference can be eliminated. Huh? Where did you get this from? Scaling works with the DRM driver. >>> >>> I was looking for the usage of scaling reg (as there is no scaling >>> command). How the scaling is implemented then? >> Like you said above the drivers work completly different. The DRM one >> receives a command list that is constructed by userspace (libdrm >> mostly), copies it to a contiguous buffer and passes the memory address >> of that buffer to the engine which then works on it. Of course >> everything is slightly more complex. >> >> You don't see any reference to scaling in the driver because the scaling >> regs don't need any kind of specific validation. >> >> If you want to know how the command list is constructed, the best way is >> to look into libdrm. The Exynos specific tests actually cover scaling. > > Thanks for explanations. The patch is insufficient then and it requires > much more effort. Please drop the series as of now. If you intend to add support for v3 hardware we might want to join forces. I have an (ongoing) rewrite of the driver to enable additional functionality and to make it more suitable to provide EXA for a X11 DDX. However this breaks the existing API and rewrites quite a bit of the driver Here's what's there so far. https://github.com/tobiasjakobi/linux-odroid-public/compare/e35ca9aca1214c5e104e6906c1d9affeb80fe5df...3d1ddb86db73b0d664f3e339709e8e8dacdc8e91 And here's the DDX: https://github.com/tobiasjakobi/xf86-video-armsoc/commits/g2d With best wishes, Tobias > > Best regards, > Krzysztof >
[PATCH 6/6] ARM: dts: sun5i: q8-common: Enable display pipeline and LCD display
On Tue, May 17, 2016 at 11:56:11PM +0800, Chen-Yu Tsai wrote: > Q8 form factor A13 tablets have a 7" LCD panel. Unfortunately we > don't know the exact model of the panel. Just pick a panel with > display timings close to what we know. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/50918e19/attachment.sig>
[PATCH 5/6] ARM: dts: sun5i: q8-common: Enable audio codec
On Tue, May 17, 2016 at 11:56:10PM +0800, Chen-Yu Tsai wrote: > The Q8 tablets use the audio codec to provide audio output via a > headphone jack or a small mono speaker. A GPIO output is used to > control speaker amp. > > The tablets may or may not have an internal microphone. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/4ba23d8c/attachment.sig>
[PATCH] drm/exynos: g2d: drop the _REG postfix from the stride defines
This makes the defines consistent with the rest.
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 01b72b9..0934d38 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -48,13 +48,13 @@
/* registers for base address */
#define G2D_SRC_BASE_ADDR 0x0304
-#define G2D_SRC_STRIDE_REG 0x0308
+#define G2D_SRC_STRIDE 0x0308
#define G2D_SRC_COLOR_MODE 0x030C
#define G2D_SRC_LEFT_TOP 0x0310
#define G2D_SRC_RIGHT_BOTTOM 0x0314
#define G2D_SRC_PLANE2_BASE_ADDR 0x0318
#define G2D_DST_BASE_ADDR 0x0404
-#define G2D_DST_STRIDE_REG 0x0408
+#define G2D_DST_STRIDE 0x0408
#define G2D_DST_COLOR_MODE 0x040C
#define G2D_DST_LEFT_TOP 0x0410
#define G2D_DST_RIGHT_BOTTOM 0x0414
@@ -560,7 +560,7 @@ static enum g2d_reg_type g2d_get_reg_type(int reg_offset)
switch (reg_offset) {
case G2D_SRC_BASE_ADDR:
- case G2D_SRC_STRIDE_REG:
+ case G2D_SRC_STRIDE:
case G2D_SRC_COLOR_MODE:
case G2D_SRC_LEFT_TOP:
case G2D_SRC_RIGHT_BOTTOM:
@@ -570,7 +570,7 @@ static enum g2d_reg_type g2d_get_reg_type(int reg_offset)
reg_type = REG_TYPE_SRC_PLANE2;
break;
case G2D_DST_BASE_ADDR:
- case G2D_DST_STRIDE_REG:
+ case G2D_DST_STRIDE:
case G2D_DST_COLOR_MODE:
case G2D_DST_LEFT_TOP:
case G2D_DST_RIGHT_BOTTOM:
@@ -961,8 +961,8 @@ static int g2d_check_reg_offset(struct device *dev,
} else
buf_info->types[reg_type] = BUF_TYPE_GEM;
break;
- case G2D_SRC_STRIDE_REG:
- case G2D_DST_STRIDE_REG:
+ case G2D_SRC_STRIDE:
+ case G2D_DST_STRIDE:
if (for_addr)
goto err;
--
2.7.3
[PATCH 4/6] ARM: dts: sun5i: q8-common: Enable USB power supply
On Tue, May 17, 2016 at 11:56:09PM +0800, Chen-Yu Tsai wrote: > On the A13 Q8 tablets, the PMIC's USB power supply (VBUS) is connected > to the external OTG port. This can be used to provide power and OTG VBUS > sensing. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/ee47c2a1/attachment.sig>
[PATCH 0/6] ARM: dts: sun5i: q8-common: Enable various peripherals
Hi Hans, On Wed, May 18, 2016 at 10:13:52AM +0200, Hans de Goede wrote: > Hi Chen-Yu, > > On 17-05-16 17:56, Chen-Yu Tsai wrote: > >Hi everyone, > > > >This series enables more peripherals for Allwinner A13-based Q8 devices. > >The peripherals include the WiFi, the PMIC's micro-usb power supply > >(driver), audio codec (headphone and speaker), and full display support > >with the new DRM KMS driver. The series is based on parts of Maxime's > >"drm: Add Support for Passive RGB to VGA bridges" series, specifically > >the clk and drm fixes, and common DTS bits. > > > >Hans, since you have a collection of q8 tablets, I'm hoping you could > >test these bits and provide some feedback. > > I've not run any tests, just looked at the patches, the stuff in > patch 3 is already present in arch/arm/boot/dts/sun5i-a13-q8-tablet.dts > I put it there because I saw some .fex files which don't use ldo3 for wifi > I'm fine with moving it to -common though, I believe that you're right > that it makes more sense there, having it in 2 places is not really > useful though. I'm not quite sure what you mean, do you want Chen-Yu to send a second version, or are you ok with this patch? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/2d9b7f8e/attachment.sig>
[PATCH 2/6] ARM: dts: axp209: Disable ldo5 by default
On Tue, May 17, 2016 at 11:56:07PM +0800, Chen-Yu Tsai wrote: > The output pin of LDO is also a GPIO pin, and the on/off settings of the > regulator are actually pinmux settings. > > Disable it by default so it doesn't conflict with GPIO usage. > > Signed-off-by: Chen-Yu Tsai Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/67730757/attachment.sig>
[PATCH 1/6] drm: sun4i: do cleanup if RGB output init fails
On Tue, May 17, 2016 at 11:56:06PM +0800, Chen-Yu Tsai wrote:
> sun4i_rgb_init() can fail, which results in TCON failing to bind.
> In this case we need to do cleanup, specificly unregistering the
> dotclock, which is regmap based, and the regmap is registered as
> part of the sun4i_tcon_bind().
>
> Failing to do so results in a NULL pointer reference when the CCF
> tries to turn off unused clocks.
>
> Fixes: 29e57fab97fc ("drm: sun4i: Add RGB output")
> Signed-off-by: Chen-Yu Tsai
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/fd557265/attachment.sig>
[PATCH] drm/exynos: remove superfluous inclusions of fbdev header
Neither of these files issue any fbdev related calls. Signed-off-by: Tobias Jakobi --- drivers/gpu/drm/exynos/exynos7_drm_decon.c | 1 - drivers/gpu/drm/exynos/exynos_drm_core.c | 1 - drivers/gpu/drm/exynos/exynos_drm_fimd.c | 1 - 3 files changed, 3 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c index da1bd84..d9b2d77 100644 --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c @@ -31,7 +31,6 @@ #include "exynos_drm_plane.h" #include "exynos_drm_drv.h" #include "exynos_drm_fb.h" -#include "exynos_drm_fbdev.h" #include "exynos_drm_iommu.h" /* diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c b/drivers/gpu/drm/exynos/exynos_drm_core.c index 011211e..edbd98f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_core.c +++ b/drivers/gpu/drm/exynos/exynos_drm_core.c @@ -15,7 +15,6 @@ #include #include "exynos_drm_drv.h" #include "exynos_drm_crtc.h" -#include "exynos_drm_fbdev.h" static LIST_HEAD(exynos_drm_subdrv_list); diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index e20a194..bc175d6 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -30,7 +30,6 @@ #include "exynos_drm_drv.h" #include "exynos_drm_fb.h" -#include "exynos_drm_fbdev.h" #include "exynos_drm_crtc.h" #include "exynos_drm_plane.h" #include "exynos_drm_iommu.h" -- 2.7.3
[PATCH 14/20] ARM: sun5i: a13: Add LCD pins
On Tue, May 17, 2016 at 01:13:01AM +0800, Chen-Yu Tsai wrote:
> > arch/arm/boot/dts/sun5i-a13.dtsi | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi
> > b/arch/arm/boot/dts/sun5i-a13.dtsi
> > index 263d46dbc7e6..79b5d513c142 100644
> > --- a/arch/arm/boot/dts/sun5i-a13.dtsi
> > +++ b/arch/arm/boot/dts/sun5i-a13.dtsi
> > @@ -237,6 +237,16 @@
> > &pio {
> > compatible = "allwinner,sun5i-a13-pinctrl";
> >
> > + lcd_rgb666_pins: lcd_rgb666 at 0 {
> > + allwinner,pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > +"PD10", "PD11", "PD12", "PD13", "PD14",
> > "PD15",
> > +"PD18", "PD19", "PD20", "PD21", "PD22",
> > "PD23",
> > +"PD24", "PD25", "PD26", "PD27";
> > + allwinner,function = "lcd0";
> > + allwinner,drive = ;
>
> Just a thought: would we ever need to increase the drive strength for very
> high
> resolutions?
I don't think we need to, but we can always change that later.
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/48adf3f9/attachment.sig>
[Bug 90284] GPU lockup with DOTA2
https://bugs.freedesktop.org/show_bug.cgi?id=90284 --- Comment #9 from Trippler --- (In reply to Oded Gabbay from comment #8) > Is this still relevant ? > We got reports it works with Ubuntu 16.04 I can confirm that I can no longer recreate the bug. They have switched the game engine the game uses since this bug was reported, so I do not know if the cause is fixed or it is no longer triggered. I do think it's safe to mark this as no longer relevant though. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/76361d11/attachment.html>
[PATCH] drm/exynos: use logical AND in exynos_drm_plane_check_size()
The current bitwise AND should result in the same assembler
but this is what the code is actually supposed to do.
Signed-off-by: Tobias Jakobi
---
drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index b3f3c2c..e4a1a63 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -226,7 +226,7 @@ exynos_drm_plane_check_size(const struct
exynos_drm_plane_config *config,
state->v_ratio == (1 << 15))
height_ok = true;
- if (width_ok & height_ok)
+ if (width_ok && height_ok)
return 0;
DRM_DEBUG_KMS("scaling mode is not supported");
--
2.7.3
fsl-dcu not works on latest "drm-next"
On Wednesday 25 May 2016 10:25:29, Meng Yi wrote:
> Hi Alexander,
> Thanks for your reply.
>
> > Commit d761701c55a99598477f3cb25c03d939a7711e74 only has one child
> > commit in my repo. Both touch only i915 related things. Please do a proper
> > bisect and name the offending commit. On which commit you got that
> > backtrace BTW?
> > From your backtrace I can't see anything related to regmap.
>
> It is weird that using bisect, for the commit log is not linear.
> I mean a newer date commit may be merged before an older date commit, when
> jump to that older date commit, the newer one will be lost, even though it
> is merged before older one. So, I think it's difficult to use git bisect.
> " d761701c55a99598477f3cb25c03d939a7711e74 " is just an older one, I mean
> between this commit and the next commit, maybe lots of commits are lost.
> So, it looks like this commit have nothing to do with the problem.
Why are commits lost? The order of commit dates is not straightforward, yes,
but that's not a problem at all.
> According to the backtrace, looks like the vblank interrupt is not happen or
> handled. Then I found the irq is installed successfully, so the problem
> seems like the vblank irq is not properly setup. And here is the point ,
> irq initia, irq handler and timing control code are using regmap.
>From your backtrace I guess wait_event_timeout is called in some atomic
context (might_sleep(); is called inside wait_event_timeout). This has nothing
to do with regmap.
> I read out the value of relevant register using "CodeWarrior TAP", find that
> endianness is not right.
>
> Then I changed endianness of the value to be written that using "
> regmap_write" . It works. But "regmap_update_bits" still have the problem.
>
> I had checked log of regmap, and didn't find which commit caused that.
The inital problem came up with 922a9f936e40001f9b921379aab90047d5990923
("regmap: mmio: Convert to regmap_bus and fix accessor usage"). The commits
9f9f8b863ad130ec0c25f378bdbad64ba71291de,
4f7d6dd4df8b388e2056c89b528254cdd79dea2a and
0dbdb76c0ca8e7caf27c9a210f64c4359e2974a4 tried to fix that. With those I could
successfully probe DCU.
Best regards,
Alexander
[Bug 95474] Bioshock Infinite and DiRT Showdown perform very poorly on any GPU with GCN >=1.1
https://bugs.freedesktop.org/show_bug.cgi?id=95474 --- Comment #13 from Jan Ziak <0xe2.0x9a.0x9b at gmail.com> --- (In reply to Marek Olšák from comment #11) > This article suggests that the regression happened between 11.2 and master. > Do you disagree with that? > http://www.phoronix.com/scan.php?page=news_item&px=RadeonSI-Padoka-May- > Ubuntu- > 16&utm_source=feedburner&utm_medium=feed&utm_campaign=Feed%3A+Phoronix+%28Pho > ronix%29 I am unable to reproduce the results from the article on my machine with Mesa 11.2.0. Mesa 11.2.0 and mesa-git have similar performance on my machine. The 37.35 FPS (Ultra quality setting) in the Phoronix article for Mesa 11.2.0 is highly unlikely. Bioshock benchmark ignores the command-line option -ForceCompatLevel=N if there exists "$HOME/.local/share/irrationalgames/bioshockinfinite". -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/8af5c49c/attachment.html>
[PATCH 1/2] drm/exynos: g2d: Add support for old S5Pv210 type
On 05/24/2016 06:05 PM, Tobias Jakobi wrote: > Hello Krzysztof, > > > Krzysztof Kozlowski wrote: >> On 05/24/2016 03:49 PM, Tobias Jakobi wrote: >>> Hello Krzysztof, >>> >>> are you sure that these are the only differences. Because AFAIK there >>> are quite a few more: >>> - DMA submission of commands >>> - blend mode / rounding >>> - solid fill >>> - YCrCb support >>> - and probably more >>> >>> One would need to add least split the command list parser into a v3 and >>> v41 version to accomodate for the differences. In fact userspace/libdrm >>> would need to know which hw type it currently uses, but you currently >>> always return 4.1 in the corresponding ioctl. >> >> Eh, so probably my patch does not cover fully the support for v3 G2D. I >> looked mostly at the differences between v3 and v4 in the s5p-g2d driver >> itself. However you are right that this might be not sufficient because >> exynos-g2d moved forward and is different than s5p-g2d. >> >>> Krzysztof Kozlowski wrote: The non-DRM s5p-g2d driver supports two versions of G2D: v3.0 on S5Pv210 and v4.x on Exynos 4x12 (or newer). The driver for 3.0 device version is doing two things differently: 1. Before starting the render process, it invalidates caches (pattern, source buffer and mask buffer). Cache control is not present on v4.x device. 2. Scalling is done through StretchEn command (in BITBLT_COMMAND_REG register) instead of SRC_SCALE_CTRL_REG as in v4.x. However the exynos_drm_g2d driver does not implement the scalling so this difference can be eliminated. >>> Huh? Where did you get this from? Scaling works with the DRM driver. >> >> I was looking for the usage of scaling reg (as there is no scaling >> command). How the scaling is implemented then? > Like you said above the drivers work completly different. The DRM one > receives a command list that is constructed by userspace (libdrm > mostly), copies it to a contiguous buffer and passes the memory address > of that buffer to the engine which then works on it. Of course > everything is slightly more complex. > > You don't see any reference to scaling in the driver because the scaling > regs don't need any kind of specific validation. > > If you want to know how the command list is constructed, the best way is > to look into libdrm. The Exynos specific tests actually cover scaling. Thanks for explanations. The patch is insufficient then and it requires much more effort. Please drop the series as of now. Best regards, Krzysztof
[PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
Thanks to Ville Syrjälä for pointing me towards the cause of this issue.
Unfortunately one of the sideaffects of having the refclk for a DPLL set
to SSC is that as long as it's set to SSC, the GPU will prevent us from
powering down any of the pipes or transcoders using it. A couple of
BIOSes enable SSC in both PCH_DREF_CONTROL and in the DPLL
configurations. This causes issues on the first modeset, since we don't
expect SSC to be left on and as a result, can't successfully power down
the pipes or the transcoders using it. Here's an example from this Dell
OptiPlex 990:
[drm:intel_modeset_init] SSC enabled by BIOS, overriding VBT which says disabled
[drm:intel_modeset_init] 2 display pipes available.
[drm:intel_update_cdclk] Current CD clock rate: 40 kHz
[drm:intel_update_max_cdclk] Max CD clock rate: 40 kHz
[drm:intel_update_max_cdclk] Max dotclock rate: 36 kHz
vgaarb: device changed decodes:
PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[drm:intel_crt_reset] crt adpa set to 0xf4
[drm:intel_dp_init_connector] Adding DP connector on port C
[drm:intel_dp_aux_init] registering DPDDC-C bus for card0-DP-1
[drm:ironlake_init_pch_refclk] has_panel 0 has_lvds 0 has_ck505 0
[drm:ironlake_init_pch_refclk] Disabling SSC entirely
⦠later we try committing the first modeset â¦
[drm:intel_dump_pipe_config] [CRTC:26][modeset] config 88041b02e800 for
pipe A
[drm:intel_dump_pipe_config] cpu_transcoder: A
â¦
[drm:intel_dump_pipe_config] dpll_hw_state: dpll: 0xc4016001, dpll_md: 0x0,
fp0: 0x20e08, fp1: 0x30d07
[drm:intel_dump_pipe_config] planes on this crtc
[drm:intel_dump_pipe_config] STANDARD PLANE:23 plane: 0.0 idx: 0 enabled
[drm:intel_dump_pipe_config] FB:42, fb = 800x600 format = 0x34325258
[drm:intel_dump_pipe_config] scaler:0 src (0, 0) 800x600 dst (0, 0) 800x600
[drm:intel_dump_pipe_config] CURSOR PLANE:25 plane: 0.1 idx: 1 disabled,
scaler_id = 0
[drm:intel_dump_pipe_config] STANDARD PLANE:27 plane: 0.1 idx: 2 disabled,
scaler_id = 0
[drm:intel_get_shared_dpll] CRTC:26 allocated PCH DPLL A
[drm:intel_get_shared_dpll] using PCH DPLL A for pipe A
[drm:ilk_audio_codec_disable] Disable audio codec on port C, pipe A
[drm:intel_disable_pipe] disabling pipe A
[ cut here ]
WARNING: CPU: 1 PID: 130 at drivers/gpu/drm/i915/intel_display.c:1146
intel_disable_pipe+0x297/0x2d0 [i915]
pipe_off wait timed out
â¦
---[ end trace 94fc8aa03ae139e8 ]---
[drm:intel_dp_link_down]
[drm:ironlake_crtc_disable [i915]] *ERROR* failed to disable transcoder A
Later modesets succeed since they reset the DPLL's configuration anyway,
but this is enough to get stuck with a big fat warning in dmesg.
A better solution would be to add refcounts for the SSC source, but for
now leaving the source clock on should suffice.
Changes since v3:
- Move temp variable into loop
- Move checks for using_ssc_source to after we've figured out has_ck505
- Add using_ssc_source to debug output
Changes since v2:
- Fix debug output for when we disable the CPU source
Changes since v1:
- Leave the SSC source clock on instead of just shutting it off on all
of the DPLL configurations.
Cc: stable at vger.kernel.org
Reviewed-by: Ville Syrjälä
Signed-off-by: Lyude
---
drivers/gpu/drm/i915/intel_display.c | 49 ++--
1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d500e6f..471e3a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8230,12 +8230,14 @@ static void ironlake_init_pch_refclk(struct drm_device
*dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *encoder;
+ int i;
u32 val, final;
bool has_lvds = false;
bool has_cpu_edp = false;
bool has_panel = false;
bool has_ck505 = false;
bool can_ssc = false;
+ bool using_ssc_source = false;
/* We need to take the global config into account */
for_each_intel_encoder(dev, encoder) {
@@ -8262,8 +8264,22 @@ static void ironlake_init_pch_refclk(struct drm_device
*dev)
can_ssc = true;
}
- DRM_DEBUG_KMS("has_panel %d has_lvds %d has_ck505 %d\n",
- has_panel, has_lvds, has_ck505);
+ /* Check if any DPLLs are using the SSC source */
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ u32 temp = I915_READ(PCH_DPLL(i));
+
+ if (!(temp & DPLL_VCO_ENABLE))
+ continue;
+
+ if ((temp & PLL_REF_INPUT_MASK) ==
+ PLLB_REF_INPUT_SPREADSPECTRUMIN) {
+ using_ssc_source = true;
+ break;
+ }
+ }
+
+ DRM_DEBUG_KMS("has_panel %d has_lvds %d has_ck505 %d using_ssc_source
%d\n",
+ has_panel, has_lvds, has_ck505, using_ssc_sourc
[PATCH 09/20] drm/sun4i: defer only if we didn't find our panel
On Tue, May 17, 2016 at 11:52:10AM +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Mon, May 16, 2016 at 8:47 PM, Maxime Ripard
> wrote:
> > Our code currently defers our probe on any error, even if we were not
> > expecting to have one at all.
> >
> > Make sure we return -EPROBE_DEFER only when we were supposed to have a
> > panel, but it's not probed yet.
> >
> > Fixes: 29e57fab97fc ("drm: sun4i: Add RGB output")
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 15 ---
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 9f19b0e08560..eed6a9e8d9a6 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -429,7 +429,7 @@ static struct drm_panel *sun4i_tcon_find_panel(struct
> > device_node *node)
> > return ERR_PTR(-EINVAL);
> > }
> >
> > - return of_drm_find_panel(remote);
> > + return of_drm_find_panel(remote) ?: ERR_PTR(-EPROBE_DEFER);
>
> There's also a typo in the DRM_DEBUG_DRIVER call a few lines up. Could you
> fix it as well?
Done, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/7695e4da/attachment.sig>
[PATCH 08/20] drm/sun4i: rgb: panel is an error pointer
On Tue, May 17, 2016 at 11:51:36AM +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Mon, May 16, 2016 at 8:47 PM, Maxime Ripard
> wrote:
> > In case of an error, our pointer to the drm_panel structure attached to our
> > encoder will hold an error pointer, not a NULL pointer.
> >
> > Make sure we check the right thing.
> >
> > Fixes: 29e57fab97fc ("drm: sun4i: Add RGB output")
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/sun4i/sun4i_rgb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > index 923f55039cb1..b46d2c15dc95 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> > @@ -217,7 +217,7 @@ int sun4i_rgb_init(struct drm_device *drm)
> > int ret;
> >
> > /* If we don't have a panel, there's no point in going on */
> > - if (!tcon->panel)
> > + if (IS_ERR(tcon->panel))
>
> Without the next patch, tcon->panel could be the result of
> of_drm_find_panel(),
> which could be NULL.
>
> Use IS_ERR_OR_NULL here, or reorder/squash the patches.
Switched the order, and applied both, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/c766743b/attachment.sig>
[PATCH 06/20] drm/sun4i: allow dclk to modify its parent rate
On Tue, May 17, 2016 at 01:18:44AM +0800, Chen-Yu Tsai wrote: > Hi, > > On Mon, May 16, 2016 at 8:47 PM, Maxime Ripard > wrote: > > The pixel clock being only a divider of its parent clock, depending on the > > resolution, it's expected to change its parent rate. Add that flag so that > > the clock framework knows it. > > This should be squashed into the previous patch. Otherwise, the previous one > doesn't really fix things, and probably makes things worse as it assumes the > parent clock would be changed. Merged the two patches and applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160525/d359e38d/attachment.sig>
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 1:20 PM, Ville Syrjälä wrote: > On Wed, May 25, 2016 at 12:36:53PM -0400, Rob Clark wrote: >> On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: >> > On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: >> >> On 05/25/2016 12:40 PM, Daniel Vetter wrote: >> >> >- Is the size/width really independent of e.g. rotation/pixel format/... >> >> > Should it be the maximum that's possible under best circumstance >> >> > (things >> >> > could still fail), or the minimum that's guaranteed to work everwhere. >> >> > This is the kind of stuff we need the userspace part for, too. >> >> >> >> Yeah, it isn't independent of these parameters. I'm not entirely sure >> >> about this either. >> >> >> >> Does it make sense to impose a rule that the user first sets the >> >> rotation/format plane properties, and only then read the maximum >> >> width? I'm assuming user space hates such kind of stuff. >> >> >> >> If we use the 'best circumstance' max_width, we can first start >> >> with a minimum number of planes that need to be grouped to achieve >> >> the target mode. If that fails the atomic test, then we can try to >> >> add one plane at a time, and reduce the width for each plane. >> >> >> >> If we use the minimum/'guaranteed to work' max_width, we'll get >> >> a higher number of planes than needed for this mode. This would pass >> >> the atomic test. We could then reduce a plane at a time and see when >> >> we fail the atomic test. >> >> >> >> I guess we need to chose the one that's more probable to get right >> >> the first time. Considering only pixel formats for now, the >> >> minimum/'guaranteed to work' would map to the RGB formats. The best >> >> circumstance ones would probably be the planar video formats. Since >> >> we use RGB more often, the minimum one might make more sense. >> >> >> >> We could, of course, give the property a range of max widths to >> >> confuse user space even more. >> > >> > An entirely different idea for cases where a simple hint property doesn't >> > work (other ideas floating around are can_scale, to give a hint whether a >> > plan can at least in theory up/downscale, or not at all), is that the >> > kernel gives more specific hints about what it would like to change. >> > >> > So if userspace asks for a plane, but for the given pixel format it's too >> > wide, the kernel could return a new proposed value for width. That would >> > be super-flexible and could cover all kinds of use-case like rotation >> > needing a specific tiling (fb_modifier) or specific pixel format, or >> > specific stride. >> > >> > For the case at hand there's even more worms: What about stride >> > requirements? Afaik on some hw you just need to split the buffers into 2 >> > planes, but can keep the wide stride (since the limit is the size of the >> > linebuffers in the hw). On others you need to split the buffer itself into >> > 2, because the plane hw can't cope with huge strides. Again might depend >> > upon the pixel format. >> > >> > So in a way height/width is both too much information and not precise >> > enough. Entirely different approches: >> > - We just add a might_need_split_plane prop to crtcs where this might be >> > needed. Userspace then gets to retry with split buffers if it doesn't >> > work with a huge one. >> > >> > - When I discussed this with qualcom folks for msm we concluded that the >> > simplest approach would be to hide this in the kernel. So if you have a >> > too wide plane, and need 2 hw planes to scan it out, then do that >> > transparently in the kernel. Of course this means that there will be 1 >> > (or 3 if you need a 2x2 split) fewer planes available, but userspace >> > needs to iteratively build up the plane config using ATOMIC_TEST anyway. >> >> Just fwiw, there are a few things that we will still end up >> abstracting in the kernel by virtualizing the mapping between kms >> planes and hw pipes. And the approach of weston atomic of >> incrementally adding more planes w/ TESTONLY flag should work well for >> that. (Let's hope the weston bits get upstream some day..) >> >> But exposing width limit avoids the one-plane to multiple-pipes case, >> considerably simplifying things. And seemed like a generic enough >> limit (iirc, it applies to omapdss and probably others), that it would >> be cleaner to expose the limit to userspace. So there should be at >> least a couple other drivers that could avoid virtualizing planes with >> some help from userspace for this case. >> >> Regarding rotation, I'm not 100% sure.. seems like we could just >> document these as the un-rotated limits. If we really had to, we >> could do some sort of dance where userspace sets rotation property on >> an un-used plane, and then reads-back the current values of the >> read-only prop's. But that seems awkward. > > I'm thinking all of this is doomed to fail. So right now people seem to > want some kind of maximum size of the source viewport. Wh
[PATCH v2 05/10] drm/rockchip: analogix_dp: add rk3399 eDP support
On 05/24/2016 06:17 PM, Heiko Stuebner wrote:
> Am Dienstag, 24. Mai 2016, 14:57:23 schrieb Yakir Yang:
> []
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 29c4105..d5d4e04
>> 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -148,6 +148,10 @@ rockchip_dp_drm_encoder_atomic_check(struct
>> drm_encoder *encoder, struct drm_connector_state *conn_state)
>> {
>> struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
>> +struct rockchip_dp_device *dp = to_dp(encoder);
>> +int ret;
>> +
>> +s->output_type = DRM_MODE_CONNECTOR_eDP;
>>
>> /*
>> * FIXME(Yakir): driver should configure the CRTC output video
>> @@ -162,8 +166,27 @@ rockchip_dp_drm_encoder_atomic_check(struct
>> drm_encoder *encoder, * But if I configure CTRC to RGBaaa, and eDP driver
>> still keep * RGB666 input video mode, then screen would works prefect.
>> */
>> -s->output_mode = ROCKCHIP_OUT_MODE_;
>> -s->output_type = DRM_MODE_CONNECTOR_eDP;
>> +
>> +ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
>> +if (ret < 0)
>> +return;
> this needs a value returned (probably ret), otherwise you create a
> warning: âreturnâ with no value, in function returning non-void
Done,
Thanks,
- Yakir
>
> drm_of_encoder_active_endpoint_id also always returns -EINVAL on rk3288-
> veyron-jerry because encoder->crtc is unset in
> drm_of_encoder_active_endpoint and that breaks display output right now.
>
> Looking through drm code it seems only two functions would set encoder->crtc
> - drm_atomic_helper_update_legacy_modeset_state
> - drm_crtc_helper_set_config
>
> drm_crtc_helper_set_config callback got dropped in the atomic-conversion and
> the other sounds to be somewhat in the legacy area.
>
> After that drm-internals get a bit confusing.
>
>
> Heiko
>
>
>
[PATCH v2 05/10] drm/rockchip: analogix_dp: add rk3399 eDP support
On 05/25/2016 02:23 AM, Heiko Stuebner wrote: > Am Dienstag, 24. Mai 2016, 11:12:20 schrieb Doug Anderson: >> Hi, >> >> On Tue, May 24, 2016 at 3:17 AM, Heiko Stuebner wrote: --- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt +++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt @@ -5,6 +5,7 @@ Required properties for dp-controller: platform specific such as: * "samsung,exynos5-dp" * "rockchip,rk3288-dp" + * "rockchip,rk3399-edp" >>> the cleanlines-freak in my likes to know if there is a difference >>> between >>> the rk3399 being called -edp here and -dp on the rk3288 :-) >>> >>> [...] >> If I was a guessing man (which I'm not, but if I was), I'd guess that >> this is because on rk3288 there was only one DP port and it was an EDP >> port. ...but since there was only one people just referred to it as >> the "DP" port and that was codified in the bindings. On rk3399 there >> are two ports: one EDP and one DP. All of a sudden we need to >> differentiate. >> >> Any better suggestions for how to deal with that? > nope - everything as it should be in that regard then - the rk3288-dp is > permanent now anyway. > > But it looks like I'm sort of blind blind and only now have seen the > separate DP controller chapter in the TRM (probably because I somehow didn't > expect a second controller). RK3399 eDP controller have cut off the DP-Audio function and DP-HDCP function, so it can't be treated as DP interfaces any more, that's why I call it "eDP". > >
objtool warning: "duplicate frame pointer save"
On Wed, May 25, 2016 at 10:14:24AM -0700, Linus Torvalds wrote: > Josh, > my current git version (with gcc 5.3.1) makes objtool warn about > "duplicate frame pointer save" in drivers/gpu/drm/vmwgfx/vmwgfx_msg.c > for both vmw_send_msg() and vmw_host_get_guestinfo(). > > The reason is that VMW_PORT_HB_OUT() uses a magic instruction sequence > (a "rep outsb") to communicate with the hypervisor (it's a virtual GPU > driver for vmware), and %rbp is part of the communication. So the > inline asm does a save-and-restore of the frame pointer around the > instruction sequence. > > I actually find the objtool warning to be quite reasonable, so it's > not exactly a false positive, since in this case it actually does > point out that the frame pointer won't be reliable over that > instruction sequence. > > But in this particular case it just ends up being the wrong thing - > the code is what it is, and %rbp just can't have the frame information > due to annoying magic calling conventions. > > Is there some way to override objtool for this situation? I hate > seeing warnings that I need to ignore, it has just too often caused me > to mistakenly ignore warnings I *should* have reacted to. > > I guess a STACK_FRAME_NON_STANDARD will shut objtool up (or just > disabling it entirely for the whole file), but I was wondering about > something more targeted that could be marked in the inline asm itself > (rather than have to mark the functions that use it) I used to have a STACKTOOL_IGNORE_INSN macro that would tell the tool to skip warnings for specific instructions in inline asm: Here's an example of it how it was used: https://lkml.kernel.org/r/c0c1a92df921961cae5cce208247bb8d8a975d6d.1450442274.git.jpoimboe at redhat.com And here's the implementation of the macro: https://lkml.kernel.org/r/cd7778181d2a5251c3dc21811fdbcaa2c16c98c3.1450442274.git.jpoimboe at redhat.com As it turns out, we didn't need the macro, so I removed it. But I can easily add something like that again to get rid of the warnings. There were objections to having "OBJTOOL" in the names of macros, so I guess I'll call it STACK_INSN_NON_STANDARD, unless anybody has a better idea. -- Josh
[Nouveau] [PATCH 9/9] drm: Turn off crtc before tearing down its data structure
Hi Daniel,
On Tue, May 24, 2016 at 11:30:42PM +0200, Daniel Vetter wrote:
> On Tue, May 24, 2016 at 06:03:27PM +0200, Lukas Wunner wrote:
> > When a drm_crtc structure is destroyed with drm_crtc_cleanup(), the DRM
> > core does not turn off the crtc first and neither do the drivers. With
> > nouveau, radeon and amdgpu, this causes a runtime pm ref to be leaked on
> > driver unload if at least one crtc was enabled.
> >
> > (See usage of have_disp_power_ref in nouveau_crtc_set_config(),
> > radeon_crtc_set_config() and amdgpu_crtc_set_config()).
> >
> > Fixes: 5addcf0a5f0f ("nouveau: add runtime PM support (v0.9)")
> > Cc: Dave Airlie
> > Tested-by: Karol Herbst
> > Signed-off-by: Lukas Wunner
>
> This is a core regression, we fixed it again. Previously when unreference
> drm_planes the core made sure that it's not longer in use, which had the
> side effect of shutting everything off in module unload.
>
> For a bunch of reasons we've stopped doing that, but that turned out to be
> a mistake. It's fixed since
>
> commit f2d580b9a8149735cbc4b59c4a8df60173658140
> Author: Maarten Lankhorst
> Date: Wed May 4 14:38:26 2016 +0200
>
> drm/core: Do not preserve framebuffer on rmfb, v4.
>
> Your patch shouldn't be needed with that any more. If it still is it's
> most likely the fbdev cleanup done too late, but you /should/ get a big
> WARNING splat in that case from drm_mode_config_cleanup().
I tested it and at least with nouveau, the above-mentioned commit does *not*
solve the issue, so patch [9/9] of this series is still needed. I do not get
a WARN splat when unloading nouveau.
Best regards,
Lukas
>
> > ---
> > drivers/gpu/drm/drm_crtc.c | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index d2a6d95..0cd6f00 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -716,12 +716,23 @@ EXPORT_SYMBOL(drm_crtc_init_with_planes);
> > *
> > * This function cleans up @crtc and removes it from the DRM mode setting
> > * core. Note that the function does *not* free the crtc structure itself,
> > - * this is the responsibility of the caller.
> > + * this is the responsibility of the caller. If @crtc is currently enabled,
> > + * it is turned off first.
> > */
> > void drm_crtc_cleanup(struct drm_crtc *crtc)
> > {
> > struct drm_device *dev = crtc->dev;
> >
> > + if (crtc->enabled) {
> > + struct drm_mode_set modeset = {
> > + .crtc = crtc,
> > + };
> > +
> > + drm_modeset_lock_all(dev);
> > + drm_mode_set_config_internal(&modeset);
> > + drm_modeset_unlock_all(dev);
> > + }
> > +
> > kfree(crtc->gamma_store);
> > crtc->gamma_store = NULL;
> >
> > --
> > 2.8.1
> >
> > ___
> > Nouveau mailing list
> > Nouveau at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
[Bug 88861] [efi, i915, vgaswitcheroo, black screen, nouveau] Screen goes black when switching from dedicated nvidia graphics card (nouveau) to integrated
https://bugzilla.kernel.org/show_bug.cgi?id=88861 --- Comment #12 from Lukas Wunner --- Thanks for the report, Wilfried. Could you please create attachments with dmesg output and Xorg output, both with and without this commit. Your kernel config and Xorg config (if any) would also be helpful. Do you have anything special configured in your Xorg config for the Intel card? Do you have any special rEFInd features or extensions enabled which might change settings of the gmux microcontroller? -- You are receiving this mail because: You are the assignee for the bug.
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: > On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: >> On 05/25/2016 12:40 PM, Daniel Vetter wrote: >> >- Is the size/width really independent of e.g. rotation/pixel format/... >> > Should it be the maximum that's possible under best circumstance (things >> > could still fail), or the minimum that's guaranteed to work everwhere. >> > This is the kind of stuff we need the userspace part for, too. >> >> Yeah, it isn't independent of these parameters. I'm not entirely sure >> about this either. >> >> Does it make sense to impose a rule that the user first sets the >> rotation/format plane properties, and only then read the maximum >> width? I'm assuming user space hates such kind of stuff. >> >> If we use the 'best circumstance' max_width, we can first start >> with a minimum number of planes that need to be grouped to achieve >> the target mode. If that fails the atomic test, then we can try to >> add one plane at a time, and reduce the width for each plane. >> >> If we use the minimum/'guaranteed to work' max_width, we'll get >> a higher number of planes than needed for this mode. This would pass >> the atomic test. We could then reduce a plane at a time and see when >> we fail the atomic test. >> >> I guess we need to chose the one that's more probable to get right >> the first time. Considering only pixel formats for now, the >> minimum/'guaranteed to work' would map to the RGB formats. The best >> circumstance ones would probably be the planar video formats. Since >> we use RGB more often, the minimum one might make more sense. >> >> We could, of course, give the property a range of max widths to >> confuse user space even more. > > An entirely different idea for cases where a simple hint property doesn't > work (other ideas floating around are can_scale, to give a hint whether a > plan can at least in theory up/downscale, or not at all), is that the > kernel gives more specific hints about what it would like to change. > > So if userspace asks for a plane, but for the given pixel format it's too > wide, the kernel could return a new proposed value for width. That would > be super-flexible and could cover all kinds of use-case like rotation > needing a specific tiling (fb_modifier) or specific pixel format, or > specific stride. > > For the case at hand there's even more worms: What about stride > requirements? Afaik on some hw you just need to split the buffers into 2 > planes, but can keep the wide stride (since the limit is the size of the > linebuffers in the hw). On others you need to split the buffer itself into > 2, because the plane hw can't cope with huge strides. Again might depend > upon the pixel format. > > So in a way height/width is both too much information and not precise > enough. Entirely different approches: > - We just add a might_need_split_plane prop to crtcs where this might be > needed. Userspace then gets to retry with split buffers if it doesn't > work with a huge one. > > - When I discussed this with qualcom folks for msm we concluded that the > simplest approach would be to hide this in the kernel. So if you have a > too wide plane, and need 2 hw planes to scan it out, then do that > transparently in the kernel. Of course this means that there will be 1 > (or 3 if you need a 2x2 split) fewer planes available, but userspace > needs to iteratively build up the plane config using ATOMIC_TEST anyway. Just fwiw, there are a few things that we will still end up abstracting in the kernel by virtualizing the mapping between kms planes and hw pipes. And the approach of weston atomic of incrementally adding more planes w/ TESTONLY flag should work well for that. (Let's hope the weston bits get upstream some day..) But exposing width limit avoids the one-plane to multiple-pipes case, considerably simplifying things. And seemed like a generic enough limit (iirc, it applies to omapdss and probably others), that it would be cleaner to expose the limit to userspace. So there should be at least a couple other drivers that could avoid virtualizing planes with some help from userspace for this case. Regarding rotation, I'm not 100% sure.. seems like we could just document these as the un-rotated limits. If we really had to, we could do some sort of dance where userspace sets rotation property on an un-used plane, and then reads-back the current values of the read-only prop's. But that seems awkward. BR, -R > If possible for your hw I'm heavily leaning towards this last approach. If > fits entirely into the current atomic design, and all the complexity is > restricted to your driver (you need to have some allocation map between > drm planes and real hw planes, but that's it). > > Thoughts? > -Daniel > >> > >> Archit >> >> > >> >Cheers, Daniel >> > >> >>Signed-off-by: Archit Taneja >> >>--- >> >> drivers/gpu/drm/drm_atomic.c | 9 + >> >> drivers/gpu/drm/drm_crtc.c | 38
[PATCH 10/14] drm/atomic-helper: Disable planes when suspending
On Wed, May 25, 2016 at 05:30:05PM +0800, Ying Liu wrote:
> On Tue, May 24, 2016 at 7:00 PM, Daniel Vetter wrote:
> > On Tue, May 24, 2016 at 06:10:49PM +0800, Liu Ying wrote:
> >> We should disable planes explicitly when suspending.
> >> Especially, this is meaningful for those display controllers which
> >> don't support active planes without relevant CRTCs being enabled.
> >>
> >> Signed-off-by: Liu Ying
> >
> > Recommended way is to call drm_atomic_helper_disable_planes_on_crtc in
> > your crtc's ->disable() callback if your hw needs this. This is a general
> > problem (test e.g. dpms), not just an issue in suspend code.
>
> It looks legacy fbdev unblank operation fails after call that function
> in ->disable(). I made the change on top of this patch set.
> If you have any idea, please help out here.
Please explain in detail what fails and how, but your patch here is
definitely not the solution. This is something your driver must be able to
handle, and which cannot be handled in all callers of ->atomic_commit.
-Daniel
>
> Regards,
> Liu Ying
>
> >
> > Also unsetting the planes from state has a semantic meaning: It unpins the
> > backing storage, which is definitely not what we want for suspend/resume.
> > -Daniel
> >
> >> ---
> >> drivers/gpu/drm/drm_atomic_helper.c | 18 +-
> >> 1 file changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 4befe25..5331d95 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -1967,7 +1967,7 @@ commit:
> >> *
> >> * Loops through all connectors, finding those that aren't turned off and
> >> then
> >> * turns them off by setting their DPMS mode to OFF and deactivating the
> >> CRTC
> >> - * that they are connected to.
> >> + * that they are connected to. The relevant planes are deactivated as
> >> well.
> >> *
> >> * This is used for example in suspend/resume to disable all currently
> >> active
> >> * functions when suspending.
> >> @@ -1997,6 +1997,7 @@ int drm_atomic_helper_disable_all(struct drm_device
> >> *dev,
> >> drm_for_each_connector(conn, dev) {
> >> struct drm_crtc *crtc = conn->state->crtc;
> >> struct drm_crtc_state *crtc_state;
> >> + struct drm_plane *plane;
> >>
> >> if (!crtc || conn->dpms != DRM_MODE_DPMS_ON)
> >> continue;
> >> @@ -2008,6 +2009,21 @@ int drm_atomic_helper_disable_all(struct drm_device
> >> *dev,
> >> }
> >>
> >> crtc_state->active = false;
> >> +
> >> + drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
> >> + struct drm_plane_state *plane_state;
> >> +
> >> + plane_state = drm_atomic_get_plane_state(state,
> >> plane);
> >> + if (IS_ERR(plane_state)) {
> >> + err = PTR_ERR(plane_state);
> >> + goto free;
> >> + }
> >> +
> >> + err = drm_atomic_set_crtc_for_plane(plane_state,
> >> NULL);
> >> + if (err != 0)
> >> + goto free;
> >> + drm_atomic_set_fb_for_plane(plane_state, NULL);
> >> + }
> >> }
> >>
> >> err = drm_atomic_commit(state);
> >> --
> >> 2.7.4
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 05/14] drm/crtc_helper: Disable and reenable primary plane in drm_helper_crtc_mode_set
On Wed, May 25, 2016 at 05:37:41PM +0800, Ying Liu wrote: > On Tue, May 24, 2016 at 6:57 PM, Daniel Vetter wrote: > > On Tue, May 24, 2016 at 06:10:44PM +0800, Liu Ying wrote: > >> Since CRTC has already been disabled in crtc_funcs->prepare(), it doesn't > >> hurt > >> to disable the primary plane in drm_helper_crtc_mode_set() before enabling > >> it > >> in drm_helper_crtc_mode_set_base(). This makes those who reject active > >> plane > >> update in plane_funcs->atomic_check() happy. > > > > How/where exactly do you blow up? > >> > >> Signed-off-by: Liu Ying > >> --- > >> drivers/gpu/drm/drm_crtc_helper.c | 9 + > >> 1 file changed, 9 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c > >> b/drivers/gpu/drm/drm_crtc_helper.c > >> index 79555d2..7fabcd7 100644 > >> --- a/drivers/gpu/drm/drm_crtc_helper.c > >> +++ b/drivers/gpu/drm/drm_crtc_helper.c > >> @@ -1013,6 +1013,15 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, > >> struct drm_display_mode *mod > >> goto out; > >> } > >> > >> + /* > >> + * It doesn't hurt to disable primary plane here since crtc is off > >> + * and we'll enable it again in drm_helper_crtc_mode_set_base() > >> + * below soon. > >> + */ > >> + ret = drm_plane_helper_disable(crtc->primary); > > > > You can't call this helper from crtc helpers. drm_plane_helper_disable > > assumes that a) you've already extracted universal plane support for the > > primary plane by registering a proper drm_plane for it b) that you're > > driver is at least partially using atomic hooks already. > > > > Both assumptions are wrong for all current users of this function. > > drm_helper_crtc_mode_set() is purely a legacy helper for legecy drivers. > > Isn't the function a transitional helper, just as the kdoc claims? > It looks all current users are sort of atomic users already. drm_helper_crtc_mode_set is exclusively used by legacy drivers. drm_plane_helper_disable can be used for transitioning to atomic. Calling the latter from the former means you break every non-transition legacy driver. -Daniel > > Regards, > Liu Ying > > > -Daniel > > > >> + if (ret) > >> + goto out; > >> + > >> swap(crtc->state, crtc_state); > >> > >> crtc_funcs->mode_set_nofb(crtc); > >> -- > >> 2.7.4 > >> > >> ___ > >> dri-devel mailing list > >> dri-devel at lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCHv2 2/3] drm/imx: convey the pixelclk-active and de-active flags from DT to the ipu-di driver
Am Mittwoch, den 25.05.2016, 08:11 +0200 schrieb Lothar WaÃmann:
[...]
> > > @@ -81,10 +87,27 @@ static int imx_pd_connector_get_modes(struct
> > > drm_connector *connector)
> > >
> > > if (np) {
> > > struct drm_display_mode *mode = drm_mode_create(connector->dev);
> > > + struct videomode vm;
> > > + int ret;
> > >
> > > if (!mode)
> > > return -EINVAL;
> > > - of_get_drm_display_mode(np, &imxpd->mode, OF_USE_NATIVE_MODE);
> > > +
> > > + ret = of_get_videomode(np, &vm, OF_USE_NATIVE_MODE);
> > > + if (ret)
> > > + return ret;
> > > + drm_display_mode_from_videomode(&vm, &imxpd->mode);
> > > + drm_mode_debug_printmodeline(&imxpd->mode);
> > > +
> > > + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > > + imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > > + if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > > + imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > > +
> > > + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> > > + imxpd->bus_flags |= DRM_BUS_FLAG_DE_LOW;
> > > + if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> > > + imxpd->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
> >
> > This could be shared between ldb and parallel drivers if you extended
> > of_get_drm_display_mode to also return bus_flags, for example. The flags
> > translation could be put into a drm_bus_flags_from_videomode helper.
> >
> I had that in mind also, but hesitated to change the generic
> of_get_drm_display_mode() to implement some i.MX specific functionality.
Since the videomode obtained from DT includes both the timing and pin
polarity information, and the drm way to express these are
drm_display_mode and bus_flags, respectively, there's nothing i.MX
specific about this.
regards
Philipp
