Re: [PATCH v4 2/6] drm/nouveau: Add NV_PRINTK_ONCE and variants
On Thu, 2018-09-06 at 17:43 -0400, Lyude Paul wrote:
> Since we're about to use this in nouveau_backlight.c. Same thing as
> DRM_WARN_ONCE, DRM_INFO_ONCE, etc...
Can you redefine this in terms of the patches I submitted
instead?
https://lore.kernel.org/patchwork/patch/979598/
https://lore.kernel.org/patchwork/patch/979601/
> Signed-off-by: Lyude Paul
> Cc: Karol Herbst
> ---
> drivers/gpu/drm/nouveau/nouveau_drv.h | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
> b/drivers/gpu/drm/nouveau/nouveau_drv.h
> index 6e1acaec3400..d9d687916553 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drv.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
> @@ -244,10 +244,12 @@ void nouveau_drm_device_remove(struct drm_device *dev);
> struct nouveau_cli *_cli = (c);\
> dev_##l(_cli->drm->dev->dev, "%s: "f, _cli->name, ##a);\
> } while(0)
> +
> #define NV_FATAL(drm,f,a...) NV_PRINTK(crit, &(drm)->client, f, ##a)
> #define NV_ERROR(drm,f,a...) NV_PRINTK(err, &(drm)->client, f, ##a)
> #define NV_WARN(drm,f,a...) NV_PRINTK(warn, &(drm)->client, f, ##a)
> #define NV_INFO(drm,f,a...) NV_PRINTK(info, &(drm)->client, f, ##a)
> +
> #define NV_DEBUG(drm,f,a...) do {
> \
> if (unlikely(drm_debug & DRM_UT_DRIVER)) \
> NV_PRINTK(info, &(drm)->client, f, ##a); \
> @@ -257,6 +259,12 @@ void nouveau_drm_device_remove(struct drm_device *dev);
> NV_PRINTK(info, &(drm)->client, f, ##a); \
> } while(0)
>
> +#define NV_PRINTK_ONCE(l,c,f,a...) NV_PRINTK(l##_once,c,f, ##a)
> +
> +#define NV_ERROR_ONCE(drm,f,a...) NV_PRINTK_ONCE(err, &(drm)->client, f, ##a)
> +#define NV_WARN_ONCE(drm,f,a...) NV_PRINTK_ONCE(warn, &(drm)->client, f, ##a)
> +#define NV_INFO_ONCE(drm,f,a...) NV_PRINTK_ONCE(info, &(drm)->client, f, ##a)
> +
> extern int nouveau_modeset;
>
> #endif
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm/amdgpu: Add macros and documentation for format modifiers.
Hopefully this answers some questions. Other parameters that affect tiling layouts are GB_ADDR_CONFIG (all chips) and MC_ARB_RAMCFG (GFX6-8 only), and those vary with each chip. Some 32bpp 1D tiling layouts are compatible across all chips (1D display tiling is the same as SW_256B_D if Bpp == 4). On GFX9, swizzle modes <= 11 are the same on all GFX9 chips. The remaining modes depend on GB_ADDR_CONFIG and are also more efficient. Bpp, number of samples, and resource type (2D/3D) affect the layout too, e.g. 3D textures silently use thick tiling on GFX9. Harvesting doesn't affect tiling layouts. The layout changes between layers/slices a little. Always use the base address of the whole image when programming the hardware. Don't assume that the 2nd layer has the same layout. > + * TODO: Can scanout really not support fastclear data? It can, but only those encoded in the DCC buffer (0/1). There is no DAL support for DCC though. > + * TODO: Do some generations share DCC format? DCC mirrors the tiling layout, so the same tiling mode means the same DCC. Take the absolute pixel address, shift it to the right, and you'll get the DCC element address. I would generally consider DCC as non-shareable because of different meanings of TILING_INDEX between chips except maybe for common GFX9 layouts. > [comments about number of bits] We could certainly represent all formats as a list of enums, but then we would need to convert the enums to the full description in drivers. GFX6-8 can use TILING_INDEX (except for stencil, let's ignore stencil). The tiling tables shouldn't change anymore because they are optimized for the hardware, and later hw doesn't have any tables. Marek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/6] drm/panel: simple: Add support for Banana Pi 7" S070WV20-CT16 panel
This panel is marketed as Banana Pi 7" LCD display. On the back is
a sticker denoting the model name S070WV20-CT16.
This is a 7" 800x480 panel connected through a 24-bit RGB interface.
However the panel only does 262k colors.
Depending on the variant, the PCB attached to the panel module either
supports DSI, or DSI + 24-bit RGB. DSI is converted to 24-bit RGB via
an onboard ICN6211 MIPI DSI - RGB bridge chip, then fed to the panel
itself.
Signed-off-by: Chen-Yu Tsai
---
.../display/panel/bananapi,s070wv20-ct16.txt | 12 +
drivers/gpu/drm/panel/panel-simple.c | 25 +++
2 files changed, 37 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
diff --git
a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
new file mode 100644
index ..35bc0c839f49
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
@@ -0,0 +1,12 @@
+Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
+
+Required properties:
+- compatible: should be "bananapi,s070wv20-ct16"
+- power-supply: see ./panel-common.txt
+
+Optional properties:
+- enable-gpios: see ./simple-panel.txt
+- backlight: see ./simple-panel.txt
+
+This binding is compatible with the simple-panel binding, which is specified
+in ./simple-panel.txt.
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 97964f7f2ace..52d6ac572825 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -772,6 +772,28 @@ static const struct panel_desc avic_tm070ddh03 = {
},
};
+static const struct drm_display_mode bananapi_s070wv20_ct16_mode = {
+ .clock = 3,
+ .hdisplay = 800,
+ .hsync_start = 800 + 40,
+ .hsync_end = 800 + 40 + 48,
+ .htotal = 800 + 40 + 48 + 40,
+ .vdisplay = 480,
+ .vsync_start = 480 + 13,
+ .vsync_end = 480 + 13 + 3,
+ .vtotal = 480 + 13 + 3 + 29,
+};
+
+static const struct panel_desc bananapi_s070wv20_ct16 = {
+ .modes = _s070wv20_ct16_mode,
+ .num_modes = 1,
+ .bpc = 6,
+ .size = {
+ .width = 154,
+ .height = 86,
+ },
+};
+
static const struct drm_display_mode boe_hv070wsa_mode = {
.clock = 40800,
.hdisplay = 1024,
@@ -2369,6 +2391,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "avic,tm070ddh03",
.data = _tm070ddh03,
+ }, {
+ .compatible = "bananapi,s070wv20-ct16",
+ .data = _s070wv20_ct16,
}, {
.compatible = "boe,hv070wsa-100",
.data = _hv070wsa
--
2.19.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/6] drm/sun4i: Support color dithering for LCD panels
Hi, This is v2 of my sun4i-drm LCD color dithering series. v1 was from back in April [1]. Most of the driver code is unchanged. Changes since v1: - Explicitly list properties from the simple-panel binding that apply to the Bananapi 7" panel - Moved an incorrectly squashed change from patch 3 to patch 1, where it belongs [1] https://patchwork.ozlabs.org/cover/900725/ Original cover letter follows: Dithering is a method of approximating a color from a mixture of other colors when the required color isn't available. It reduces color banding artifacts that can be observed when displaying gradients (e.g. grayscale gradients). This may occur when the image that needs to be displayed is 24-bit but the LCD panel is a lower bit depth and does not perform dithering on its own. The TCON (LCD controller) found in Allwinner SoCs has hardware support for dithering on channel 0, the channel used to feed LCD panels. This series adds support for it. Patch 1 reworks the mode set function for the CPU interface to pass the encoder object, so it can be passed to other helper functions. Patch 2 renames the dithering related register macros to reflect the fact that dithering is only supported on channel 0. Patch 3 adds support for dithering on all LCD panel output types. Patch 4 adds support for Banana Pi's 7" DPI LCD panel. Patch 5 adds a pinmux setting for RGB888 for the Allwinner A20 SoC. This change has been sent by others before. Patch 6 provides an example for enabling the Banana Pi 7" DPI LCD panel on the Banana Pi M1+. This should not be merged. I will likely rework this into an overlay in the future. Note that I was only able to test dithering with DPI, as I do not have other panel types. However the underlying concept and core code is the same, as are the drm objects accessed. Nevertheless I'm hoping Jonathan can test LVDS and Maxime can test MIPI DSI. Also it seems pwm-backlight hardware is unusable at the moment. I'm not sure whether the pwm-backlight or sun4i-pwm driver is to blame. I had to manually poke the pwm registers so the LCD backlight wouldn't be completely black. Regards ChenYu Chen-Yu Tsai (5): drm/sun4i: tcon: Pass drm_encoder * into sun4i_tcon0_mode_set_cpu drm/sun4i: tcon: Rename Dithering related register macros drm/panel: simple: Add support for Banana Pi 7" S070WV20-CT16 panel ARM: dts: sun7i: add pinmux setting for RGB888 output for LCD0 [DO NOT MERGE] ARM: dts: sun7i: bananapi-m1-plus: Enable Bananapi 7" 800x480 RGB LCD panel Jonathan Liu (1): drm/sun4i: tcon: Add dithering support for RGB565/RGB666 LCD panels .../display/panel/bananapi,s070wv20-ct16.txt | 12 +++ .../boot/dts/sun7i-a20-bananapi-m1-plus.dts | 56 ++ arch/arm/boot/dts/sun7i-a20.dtsi | 11 +++ drivers/gpu/drm/panel/panel-simple.c | 25 ++ drivers/gpu/drm/sun4i/sun4i_tcon.c| 76 --- drivers/gpu/drm/sun4i/sun4i_tcon.h| 27 --- 6 files changed, 186 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt -- 2.19.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/6] drm/sun4i: tcon: Add dithering support for RGB565/RGB666 LCD panels
From: Jonathan Liu
The hardware supports dithering on TCON channel 0 which is used for LCD
panels.
Dithering is a method of approximating a color from a mixture of other
colors when the required color isn't available. It reduces color
banding artifacts that can be observed when displaying gradients
(e.g. grayscale gradients). This may occur when the image that needs
to be displayed is 24-bit but the LCD panel is a lower bit depth and
does not perform dithering on its own.
Signed-off-by: Jonathan Liu
[w...@csie.org: check display_info.bpc first; handle LVDS and MIPI DSI]
Signed-off-by: Chen-Yu Tsai
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 61 ++
1 file changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index d6f9d5f3b15b..4834c90b4912 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -12,6 +12,7 @@
#include
#include
+#include
#include
#include
#include
@@ -277,6 +278,57 @@ static void sun4i_tcon0_mode_set_common(struct sun4i_tcon
*tcon,
SUN4I_TCON0_BASIC0_Y(mode->crtc_vdisplay));
}
+static void sun4i_tcon0_mode_set_dithering(struct sun4i_tcon *tcon,
+ const struct drm_connector
*connector)
+{
+ u32 bus_format = 0;
+ u32 val = 0;
+
+ /* XXX Would this ever happen? */
+ if (!connector)
+ return;
+
+ /*
+* FIXME: Undocumented bits
+*
+* The whole dithering process and these parameters are not
+* explained in the vendor documents or BSP kernel code.
+*/
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED_PR_REG, 0x);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED_PG_REG, 0x);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED_PB_REG, 0x);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED_LR_REG, 0x);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED_LG_REG, 0x);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED_LB_REG, 0x);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_TBL0_REG, 0x0101);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_TBL1_REG, 0x1515);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_TBL2_REG, 0x5757);
+ regmap_write(tcon->regs, SUN4I_TCON0_FRM_TBL3_REG, 0x7f7f);
+
+ /* Do dithering if panel only supports 6 bits per color */
+ if (connector->display_info.bpc == 6)
+ val |= SUN4I_TCON0_FRM_CTL_EN;
+
+ if (connector->display_info.num_bus_formats == 1)
+ bus_format = connector->display_info.bus_formats[0];
+
+ /* Check the connection format */
+ switch (bus_format) {
+ case MEDIA_BUS_FMT_RGB565_1X16:
+ /* R and B components are only 5 bits deep */
+ val |= SUN4I_TCON0_FRM_CTL_MODE_R;
+ val |= SUN4I_TCON0_FRM_CTL_MODE_B;
+ case MEDIA_BUS_FMT_RGB666_1X18:
+ case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+ /* Fall through: enable dithering */
+ val |= SUN4I_TCON0_FRM_CTL_EN;
+ break;
+ }
+
+ /* Write dithering settings */
+ regmap_write(tcon->regs, SUN4I_TCON_FRM_CTL_REG, val);
+}
+
static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder,
const struct drm_display_mode *mode)
@@ -294,6 +346,9 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon
*tcon,
sun4i_tcon0_mode_set_common(tcon, mode);
+ /* Set dithering if needed */
+ sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
+
regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
SUN4I_TCON0_CTL_IF_MASK,
SUN4I_TCON0_CTL_IF_8080);
@@ -359,6 +414,9 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon
*tcon,
tcon->dclk_max_div = 7;
sun4i_tcon0_mode_set_common(tcon, mode);
+ /* Set dithering if needed */
+ sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
+
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
@@ -432,6 +490,9 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon
*tcon,
tcon->dclk_max_div = 127;
sun4i_tcon0_mode_set_common(tcon, mode);
+ /* Set dithering if needed */
+ sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
+
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
--
2.19.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/6] drm/sun4i: tcon: Rename Dithering related register macros
Dithering is only supported for TCON channel 0. Throughout the datasheet all the names associated with these register are prefixed "TCON0", instead of "TCON". The only exception is the control register "TCON_FRM_CTL_REG". Rename the macros to reflect this. Signed-off-by: Chen-Yu Tsai --- drivers/gpu/drm/sun4i/sun4i_tcon.h | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index f6a071cd5a6f..3d492c8be1fc 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -37,18 +37,21 @@ #define SUN4I_TCON_GINT1_REG 0x8 #define SUN4I_TCON_FRM_CTL_REG 0x10 -#define SUN4I_TCON_FRM_CTL_EN BIT(31) - -#define SUN4I_TCON_FRM_SEED_PR_REG 0x14 -#define SUN4I_TCON_FRM_SEED_PG_REG 0x18 -#define SUN4I_TCON_FRM_SEED_PB_REG 0x1c -#define SUN4I_TCON_FRM_SEED_LR_REG 0x20 -#define SUN4I_TCON_FRM_SEED_LG_REG 0x24 -#define SUN4I_TCON_FRM_SEED_LB_REG 0x28 -#define SUN4I_TCON_FRM_TBL0_REG0x2c -#define SUN4I_TCON_FRM_TBL1_REG0x30 -#define SUN4I_TCON_FRM_TBL2_REG0x34 -#define SUN4I_TCON_FRM_TBL3_REG0x38 +#define SUN4I_TCON0_FRM_CTL_EN BIT(31) +#define SUN4I_TCON0_FRM_CTL_MODE_R BIT(6) +#define SUN4I_TCON0_FRM_CTL_MODE_G BIT(5) +#define SUN4I_TCON0_FRM_CTL_MODE_B BIT(4) + +#define SUN4I_TCON0_FRM_SEED_PR_REG0x14 +#define SUN4I_TCON0_FRM_SEED_PG_REG0x18 +#define SUN4I_TCON0_FRM_SEED_PB_REG0x1c +#define SUN4I_TCON0_FRM_SEED_LR_REG0x20 +#define SUN4I_TCON0_FRM_SEED_LG_REG0x24 +#define SUN4I_TCON0_FRM_SEED_LB_REG0x28 +#define SUN4I_TCON0_FRM_TBL0_REG 0x2c +#define SUN4I_TCON0_FRM_TBL1_REG 0x30 +#define SUN4I_TCON0_FRM_TBL2_REG 0x34 +#define SUN4I_TCON0_FRM_TBL3_REG 0x38 #define SUN4I_TCON0_CTL_REG0x40 #define SUN4I_TCON0_CTL_TCON_ENABLEBIT(31) -- 2.19.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 6/6] [DO NOT MERGE] ARM: dts: sun7i: bananapi-m1-plus: Enable Bananapi 7" 800x480 RGB LCD panel
The BPI-M1+ has an FPC connector for connecting an RGB (parallel) or
LVDS LCD panel. One of the compatible panels is a 7" 800x480 RGB panel
from Bananapi. The backlight can be controlled by driving a PWM signal
from the SoC at different duty cycles. The LCD enable pin does not seem
to do anything, but it is nevertheless included for completeness. There
is also a FT5306 capacitive touchscreen controller.
This should not be confused with the other 7" LCD that is LVDS based
and has a resolution of 1024x600.
This patch enables all of the above for the BPI-M1+.
Signed-off-by: Chen-Yu Tsai
---
.../boot/dts/sun7i-a20-bananapi-m1-plus.dts | 56 +++
1 file changed, 56 insertions(+)
diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi-m1-plus.dts
b/arch/arm/boot/dts/sun7i-a20-bananapi-m1-plus.dts
index 763cb03033c4..9fa58adade56 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi-m1-plus.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi-m1-plus.dts
@@ -47,6 +47,7 @@
#include "sunxi-common-regulators.dtsi"
#include
#include
+#include
/ {
model = "Banana Pi BPI-M1-Plus";
@@ -71,6 +72,29 @@
};
};
+ lcd_backlight: lcd-backlight {
+ compatible = "pwm-backlight";
+ pwms = < 0 2 PWM_POLARITY_INVERTED>; /* 50 kHz */
+ enable-gpios = < 7 9 GPIO_ACTIVE_HIGH>; /* PH9 */
+ brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
+ default-brightness-level = <8>;
+ };
+
+ lcd_panel: lcd-panel {
+ compatible = "bananapi,s070wv20-ct16", "simple-panel";
+ power-supply = <_vcc5v0>; /* Actually driven from IPSOUT */
+ /* This doesn't do anything for this particular connector */
+ enable-gpios = < 7 12 GPIO_ACTIVE_HIGH>; /* PH12 */
+
+ port {
+ backlight = <_backlight>;
+
+ lcd_panel_input: endpoint {
+ remote-endpoint = <_out_lcd>;
+ };
+ };
+ };
+
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
@@ -173,6 +197,20 @@
status = "okay";
};
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins_a>;
+ status = "okay";
+
+ touchscreen@38 {
+ compatible = "edt,edt-ft5306", "edt,edt-ft5x06";
+ reg = <0x38>;
+ interrupt-parent = <>;
+ interrupts = <7 7 IRQ_TYPE_EDGE_FALLING>; /* PH7 */
+ reset-gpios = < 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
+ };
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_rx_pins_a>;
@@ -249,6 +287,12 @@
};
};
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins_a>;
+ status = "okay";
+};
+
_dcdc2 {
regulator-always-on;
regulator-min-microvolt = <100>;
@@ -278,6 +322,18 @@
status = "okay";
};
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_rgb888_pins>;
+};
+
+_out {
+ tcon0_out_lcd: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <_panel_input>;
+ };
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
--
2.19.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/6] ARM: dts: sun7i: add pinmux setting for RGB888 output for LCD0
On the A20, as well as many other Allwinner SoCs, the PD pingroup has
the LCD0 RGB output functions.
Add a pinmux setting for RGB888 output from LCD0, so boards and tablets
with parallel RGB LCD panels may reference it.
Signed-off-by: Chen-Yu Tsai
---
arch/arm/boot/dts/sun7i-a20.dtsi | 11 +++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 9c52712af241..b2c56833b0e6 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -950,6 +950,17 @@
pins = "PI20", "PI21";
function = "uart7";
};
+
+ lcd0_rgb888_pins: lcd0-rgb888-pins {
+ pins = "PD0", "PD1", "PD2", "PD3", "PD4",
+ "PD5", "PD6", "PD7", "PD8", "PD9",
+ "PD10", "PD11", "PD12", "PD13",
+ "PD14", "PD15", "PD16", "PD17",
+ "PD18", "PD19", "PD20", "PD21",
+ "PD22", "PD23", "PD24", "PD25",
+ "PD26", "PD27";
+ function = "lcd0";
+ };
};
timer@1c20c00 {
--
2.19.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/6] drm/sun4i: tcon: Pass drm_encoder * into sun4i_tcon0_mode_set_cpu
sun4i_tcon0_mode_set_cpu() currently accepts struct mipi_dsi_device *
as its second parameter. This is derived from drm_encoder.
The DSI encoder is tied to the CPU interface mode of the TCON as a
special case. In theory, if hardware were available, we could also
support normal CPU interface modes. It is better to pass the generic
encoder instead of the specialized mipi_dsi_device, and handle the
differences inside the function.
Passing the encoder would also enable the function to pass it, or any
other data structures related to it, to other functions expecting it.
One such example would be dithering support that will be added in a
later patch, which looks at properties tied to the connector to
determine whether dithering should be enabled or not.
Signed-off-by: Chen-Yu Tsai
---
drivers/gpu/drm/sun4i/sun4i_tcon.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 0cebb2db5b99..d6f9d5f3b15b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -278,9 +278,12 @@ static void sun4i_tcon0_mode_set_common(struct sun4i_tcon
*tcon,
}
static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon *tcon,
-struct mipi_dsi_device *device,
+const struct drm_encoder *encoder,
const struct drm_display_mode *mode)
{
+ /* TODO support normal CPU interface modes */
+ struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
+ struct mipi_dsi_device *device = dsi->device;
u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
u8 lanes = device->lanes;
u32 block_space, start_delay;
@@ -610,16 +613,10 @@ void sun4i_tcon_mode_set(struct sun4i_tcon *tcon,
const struct drm_encoder *encoder,
const struct drm_display_mode *mode)
{
- struct sun6i_dsi *dsi;
-
switch (encoder->encoder_type) {
case DRM_MODE_ENCODER_DSI:
- /*
-* This is not really elegant, but it's the "cleaner"
-* way I could think of...
-*/
- dsi = encoder_to_sun6i_dsi(encoder);
- sun4i_tcon0_mode_set_cpu(tcon, dsi->device, mode);
+ /* DSI is tied to special case of CPU interface */
+ sun4i_tcon0_mode_set_cpu(tcon, encoder, mode);
break;
case DRM_MODE_ENCODER_LVDS:
sun4i_tcon0_mode_set_lvds(tcon, encoder, mode);
--
2.19.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/13] drm/mediatek: add hdmi driver for MT2701 and MT7623
Hi, Bibby:
On Wed, 2018-09-05 at 16:31 +0800, Bibby Hsieh wrote:
> From: chunhui dai
>
> This patch adds hdmi dirver suppot for both MT2701 and MT7623.
> And also support other (existing or future) chips that use
> the same binding and driver.
>
> Signed-off-by: chunhui dai
> ---
> drivers/gpu/drm/mediatek/Makefile | 1 +
> drivers/gpu/drm/mediatek/mtk_hdmi.h| 1 +
> drivers/gpu/drm/mediatek/mtk_hdmi_phy.c| 3 +
> drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c | 307
> +
> 4 files changed, 312 insertions(+)
> create mode 100644 drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
>
> diff --git a/drivers/gpu/drm/mediatek/Makefile
> b/drivers/gpu/drm/mediatek/Makefile
> index 7f947979d68f..bf0067b5ee6f 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -2,6 +2,7 @@
> mediatek-drm-hdmi-objs := mtk_cec.o \
> mtk_hdmi.o \
> mtk_hdmi_ddc.o \
> + mtk_mt2701_hdmi_phy.o \
> mtk_mt8173_hdmi_phy.o \
> mtk_hdmi_phy.o
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.h
> b/drivers/gpu/drm/mediatek/mtk_hdmi.h
> index a350a6c9271f..fa12eb6288f3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.h
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.h
> @@ -45,5 +45,6 @@ extern struct platform_driver mtk_cec_driver;
> extern struct platform_driver mtk_hdmi_ddc_driver;
> extern struct platform_driver mtk_hdmi_phy_driver;
> extern struct mtk_hdmi_phy_conf mtk_hdmi_phy_8173_conf;
> +extern struct mtk_hdmi_phy_conf mtk_hdmi_phy_2701_conf;
>
> #endif /* _MTK_HDMI_CTRL_H */
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
> b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
> index 82ed73575e04..606fc7a0c13b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
> @@ -135,6 +135,9 @@ static int mtk_hdmi_phy_remove(struct platform_device
> *pdev)
> }
>
> static const struct of_device_id mtk_hdmi_phy_match[] = {
> + { .compatible = "mediatek,mt2701-hdmi-phy",
> + .data = _hdmi_phy_2701_conf,
> + },
> { .compatible = "mediatek,mt8173-hdmi-phy",
> .data = _hdmi_phy_8173_conf,
> },
> diff --git a/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
> b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
> new file mode 100644
> index ..428ef1557a14
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
> @@ -0,0 +1,307 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Chunhui Dai
> + */
> +
> +#include
> +#include
> +#include
Why need debugfs.h?
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "mtk_hdmi.h"
I don't know why the included file is different in mtk_mt8173_hdmi_phy.c
with this file. The difference of these two files are hardware control,
so I think these two files would include the same header files. If these
two file include the same files, maybe we should move the common include
statement in some header file to prevent duplicate in these two files.
> +
> +#define HDMI_CON00x00
> +#define RG_HDMITX_DRV_IBIAS (0)
You need not to embrace a single number.
> +#define RG_HDMITX_DRV_IBIAS_MASK (0x3F << 0)
Use lower case for hex number.
> +#define RG_HDMITX_EN_SER (12)
> +#define RG_HDMITX_EN_SER_MASK(0x0F << 12)
> +#define RG_HDMITX_EN_SLDO(16)
> +#define RG_HDMITX_EN_SLDO_MASK (0x0F << 16)
> +#define RG_HDMITX_EN_PRED(20)
> +#define RG_HDMITX_EN_PRED_MASK (0x0F << 20)
> +#define RG_HDMITX_EN_IMP (24)
> +#define RG_HDMITX_EN_IMP_MASK(0x0F << 24)
> +#define RG_HDMITX_EN_DRV (28)
> +#define RG_HDMITX_EN_DRV_MASK(0x0F << 28)
> +
> +#define HDMI_CON10x04
> +#define RG_HDMITX_PRED_IBIAS (18)
> +#define RG_HDMITX_PRED_IBIAS_MASK(0x0F << 18)
> +#define RG_HDMITX_PRED_IMP (0x01 << 22)
> +#define RG_HDMITX_DRV_IMP(26)
> +#define RG_HDMITX_DRV_IMP_MASK (0x3F << 26)
> +
> +#define HDMI_CON20x08
> +#define RG_HDMITX_EN_TX_CKLDO(0x01 << 0)
> +#define RG_HDMITX_EN_TX_POSDIV (0x01 << 1)
> +#define RG_HDMITX_TX_POSDIV (3)
> +#define RG_HDMITX_TX_POSDIV_MASK (0x03 << 3)
> +#define RG_HDMITX_EN_MBIAS (0x01 << 6)
> +#define RG_HDMITX_MBIAS_LPF_EN (0x01 << 7)
> +
> +#define HDMI_CON40x10
> +#define RG_HDMITX_RESERVE_MASK (0x << 0)
> +
> +#define HDMI_CON60x18
> +#define RG_HTPLL_BR (0)
> +#define RG_HTPLL_BR_MASK (0x03 << 0)
> +#define RG_HTPLL_BC (2)
> +#define RG_HTPLL_BC_MASK (0x03 << 2)
> +#define RG_HTPLL_BP
[git pull] drm fixes for 4.19-rc3
Hi Linus, Seems to have been overly quiet this week so I expect next week will be more stuff, just one pull request with i915 fixes in it. i915: (from Rodrigo's pull) "The critical fix here on display side is the DP MST regression one. But this pull also include fixes for DP SST, small VDSC register fix and GVT's bucked with "BXT fixes, two guest warning fixes, dmabuf format mod fix and one for recent multiple VM timeout failure." Dave. drm-fixes-2018-09-07: i915 fixes The following changes since commit 57361846b52bc686112da6ca5368d11210796804: Linux 4.19-rc2 (2018-09-02 14:37:30 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2018-09-07 for you to fetch changes up to 67c6ed7cf9ebe53815f15bfdeb49ad91801c2235: Merge tag 'drm-intel-fixes-2018-09-05' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2018-09-07 11:07:03 +1000) i915 fixes Colin Xu (2): drm/i915/gvt: Make correct handling to vreg BXT_PHY_CTL_FAMILY drm/i915/gvt: Handle GEN9_WM_CHICKEN3 with F_CMD_ACCESS. Dave Airlie (1): Merge tag 'drm-intel-fixes-2018-09-05' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Hang Yuan (1): drm/i915/gvt: move intel_runtime_pm_get out of spin_lock in stop_schedule Imre Deak (1): drm/i915/dp_mst: Fix enabling pipe clock for all streams Jan-Marek Glogowski (1): drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Manasi Navare (1): drm/i915/dsc: Fix PPS register definition macros for 2nd VDSC engine Rodrigo Vivi (1): Merge tag 'gvt-fixes-2018-09-04' of https://github.com/intel/gvt-linux into drm-intel-fixes Xiaolin Zhang (1): drm/i915/gvt: emulate gen9 dbuf ctl register access Zhenyu Wang (2): drm/i915/gvt: Fix drm_format_mod value for vGPU plane drm/i915/gvt: Give new born vGPU higher scheduling chance drivers/gpu/drm/i915/gvt/dmabuf.c | 33 ++--- drivers/gpu/drm/i915/gvt/fb_decoder.c | 5 ++--- drivers/gpu/drm/i915/gvt/fb_decoder.h | 2 +- drivers/gpu/drm/i915/gvt/handlers.c | 33 +++-- drivers/gpu/drm/i915/gvt/mmio_context.c | 2 -- drivers/gpu/drm/i915/gvt/sched_policy.c | 37 ++--- drivers/gpu/drm/i915/i915_reg.h | 4 ++-- drivers/gpu/drm/i915/intel_ddi.c| 17 --- drivers/gpu/drm/i915/intel_dp.c | 33 - drivers/gpu/drm/i915/intel_dp_mst.c | 4 10 files changed, 120 insertions(+), 50 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 07/13] drm/mediatek: separae hdmi phy to different file
Hi, Bibby:
On Wed, 2018-09-05 at 16:31 +0800, Bibby Hsieh wrote:
> From: chunhui dai
>
> Different IC has different phy setting of HDMI.
> This patch separaes the phy hardware relate part for mt8173.
I guess 'separae' is 'separate'. Am I right?
Regards,
CK
>
> Signed-off-by: chunhui dai
> ---
> drivers/gpu/drm/mediatek/Makefile | 15 +--
> drivers/gpu/drm/mediatek/mtk_hdmi.c| 30 +++--
> drivers/gpu/drm/mediatek/mtk_hdmi.h| 26 +
> drivers/gpu/drm/mediatek/mtk_hdmi_phy.c| 154
> +
> drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c | 129 ++---
> 5 files changed, 216 insertions(+), 138 deletions(-)
> create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
>
> diff --git a/drivers/gpu/drm/mediatek/Makefile
> b/drivers/gpu/drm/mediatek/Makefile
> index ce83c396a742..7f947979d68f 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -1,4 +1,12 @@
> # SPDX-License-Identifier: GPL-2.0
> +mediatek-drm-hdmi-objs := mtk_cec.o \
> + mtk_hdmi.o \
> + mtk_hdmi_ddc.o \
> + mtk_mt8173_hdmi_phy.o \
> + mtk_hdmi_phy.o
> +
> +obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
> +
> mediatek-drm-y := mtk_disp_color.o \
> mtk_disp_ovl.o \
> mtk_disp_rdma.o \
> @@ -14,10 +22,3 @@ mediatek-drm-y := mtk_disp_color.o \
> mtk_dpi.o
>
> obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
> -
> -mediatek-drm-hdmi-objs := mtk_cec.o \
> - mtk_hdmi.o \
> - mtk_hdmi_ddc.o \
> - mtk_mt8173_hdmi_phy.o
> -
> -obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 2d45d1dd9554..7c022f3f53ec 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -233,6 +233,7 @@ static void mtk_hdmi_hw_vid_black(struct mtk_hdmi *hdmi,
> bool black)
> static void mtk_hdmi_hw_make_reg_writable(struct mtk_hdmi *hdmi, bool enable)
> {
> struct arm_smccc_res res;
> + struct mtk_hdmi_phy *hdmi_phy = phy_get_drvdata(hdmi->phy);
>
> /*
>* MT8173 HDMI hardware has an output control bit to enable/disable HDMI
> @@ -240,8 +241,13 @@ static void mtk_hdmi_hw_make_reg_writable(struct
> mtk_hdmi *hdmi, bool enable)
>* The ARM trusted firmware provides an API for the HDMI driver to set
>* this control bit to enable HDMI output in supervisor mode.
>*/
> - arm_smccc_smc(MTK_SIP_SET_AUTHORIZED_SECURE_REG, 0x14000904, 0x8000,
> - 0, 0, 0, 0, 0, );
> + if (hdmi_phy->conf && hdmi_phy->conf->tz_enabled)
> + arm_smccc_smc(MTK_SIP_SET_AUTHORIZED_SECURE_REG, 0x14000904,
> + 0x8000, 0, 0, 0, 0, 0, );
> + else
> + regmap_update_bits(hdmi->sys_regmap,
> +hdmi->sys_offset + HDMI_SYS_CFG20,
> +0x80008005, enable ? 0x8005 : 0x8000);
>
> regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG20,
> HDMI_PCLK_FREE_RUN, enable ? HDMI_PCLK_FREE_RUN : 0);
> @@ -1437,6 +1443,7 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi
> *hdmi,
> struct platform_device *cec_pdev;
> struct regmap *regmap;
> struct resource *mem;
> + const char *phy_name;
> int ret;
>
> ret = mtk_hdmi_get_all_clk(hdmi, np);
> @@ -1445,6 +1452,18 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi
> *hdmi,
> return ret;
> }
>
> + ret = of_property_read_string(np, "phy-names", _name);
> + if (ret < 0) {
> + dev_err(dev, "Failed to read phy-names: %d\n", ret);
> + return ret;
> + }
> + hdmi->phy = devm_phy_get(dev, phy_name);
> + if (IS_ERR(hdmi->phy)) {
> + ret = PTR_ERR(hdmi->phy);
> + dev_err(dev, "Failed to get HDMI PHY: %d\n", ret);
> + return ret;
> + }
> +
> /* The CEC module handles HDMI hotplug detection */
> cec_np = of_find_compatible_node(np->parent, NULL,
>"mediatek,mt8173-cec");
> @@ -1677,13 +1696,6 @@ static int mtk_drm_hdmi_probe(struct platform_device
> *pdev)
> if (ret)
> return ret;
>
> - hdmi->phy = devm_phy_get(dev, "hdmi");
> - if (IS_ERR(hdmi->phy)) {
> - ret = PTR_ERR(hdmi->phy);
> - dev_err(dev, "Failed to get HDMI PHY: %d\n", ret);
> - return ret;
> - }
> -
> platform_set_drvdata(pdev, hdmi);
>
> ret = mtk_hdmi_output_init(hdmi);
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.h
> b/drivers/gpu/drm/mediatek/mtk_hdmi.h
> index 6371b3de1ff6..a350a6c9271f 100644
> ---
Re: [PATCH 1/3] drm: drm_fourcc: add Samsung 16x16 tile format
Hi,
2018년 08월 27일 09:38에 Inki Dae 이(가) 쓴 글:
>
>
> 2018년 08월 10일 22:28에 Marek Szyprowski 이(가) 쓴 글:
>> From: Andrzej Pietrasiewicz
>>
>> Add modifier for tiled formats used by graphics modules found in Samsung
>> Exynos5250/542x/5433 SoCs. This is a simple tiled layout using tiles
>> of 16x16 pixels in a row-major layout.
>
> Reviewed-by: Inki Dae
>
This patch is required by below two patches which add Samsung 16x16 tile format
support to gsc and scaler drivers.
https://patchwork.kernel.org/patch/10562707/
https://patchwork.kernel.org/patch/10562713/
Could you give me ack-by so that I could request GIT-PULL together?
Thanks,
InkI Dae
> Thanks,
> Inki Dae
>
>>
>> Signed-off-by: Andrzej Pietrasiewicz
>> Signed-off-by: Marek Szyprowski
>> ---
>> include/uapi/drm/drm_fourcc.h | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index 721ab7e54d96..5631b196c07a 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -299,6 +299,15 @@ extern "C" {
>> */
>> #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1)
>>
>> +/*
>> + * Tiled, 16 (pixels) x 16 (lines) - sized macroblocks
>> + *
>> + * This is a simple tiled layout using tiles of 16x16 pixels in a row-major
>> + * layout. For YCbCr formats Cb/Cr components are taken in such a way that
>> + * they correspond to their 16x16 luma block.
>> + */
>> +#define DRM_FORMAT_MOD_SAMSUNG_16_16_TILE fourcc_mod_code(SAMSUNG, 2)
>> +
>> /*
>> * Qualcomm Compressed Format
>> *
>>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tegra: return with probe defer if GPIO subsystem is not ready
On 26.07.2018 06:36, Stefan Agner wrote:
> If the GPIO subsystem is not ready make sure to return -EPROBE_DEFER
> instead of silently continuing without HPD.
>
> Reported-by: Marcel Ziswiler
> Signed-off-by: Stefan Agner
I think this did not get merged yet, any chance to get it in?
--
Stefan
> ---
> drivers/gpu/drm/tegra/output.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index ffe34bd0bb9d..4bcefe455afd 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -133,7 +133,9 @@ int tegra_output_probe(struct tegra_output *output)
> output->hpd_gpio = of_get_named_gpio_flags(output->of_node,
> "nvidia,hpd-gpio", 0,
> >hpd_gpio_flags);
> - if (gpio_is_valid(output->hpd_gpio)) {
> + if (output->hpd_gpio == -EPROBE_DEFER) {
> + return -EPROBE_DEFER;
> + } else if (gpio_is_valid(output->hpd_gpio)) {
> unsigned long flags;
>
> err = gpio_request_one(output->hpd_gpio, GPIOF_DIR_IN,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 01/14] drm: add new plane property FB_DAMAGE_CLIPS to send damage during plane update
>
> On Wed, Sep 05, 2018 at 04:38:48PM -0700, Deepak Rawat wrote:
> > From: Lukasz Spintzyk
> >
> > FB_DAMAGE_CLIPS is an optional plane property to mark damaged regions
> > on the plane in framebuffer coordinates of the framebuffer attached to
> > the plane.
> >
> > The layout of blob data is simply an array of "struct drm_mode_rect"
> > with maximum array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS.
> Unlike
> > plane src coordinates, damage clips are not in 16.16 fixed point. As
> > plane src in framebuffer cannot be negative so are damage clips. In
> > damage clip, x1/y1 are inclusive and x2/y2 are exclusive.
> >
> > This patch also exports the kernel internal drm_rect to userspace as
> > drm_mode_rect. This is because "struct drm_clip_rect" is not sufficient
> > to represent damage for current plane size.
> >
> > Upper limit is set on the maximum number of damage clips to avoid
> > overflow by user-space.
> >
> > Driver which are interested in enabling FB_DAMAGE_CLIPS property for a
> > plane should enable this property using drm_plane_enable_damage_clips.
> >
> > Signed-off-by: Lukasz Spintzyk
> > Signed-off-by: Deepak Rawat
> > ---
> > Documentation/gpu/drm-kms.rst | 9 +++
> > drivers/gpu/drm/Makefile| 2 +-
> > drivers/gpu/drm/drm_atomic.c| 13
> > drivers/gpu/drm/drm_atomic_helper.c | 4 ++
> > drivers/gpu/drm/drm_damage_helper.c | 92
> +
> > drivers/gpu/drm/drm_mode_config.c | 6 ++
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 1 +
> > include/drm/drm_damage_helper.h | 63
> > include/drm/drm_mode_config.h | 10
> > include/drm/drm_plane.h | 8 +++
> > include/uapi/drm/drm_mode.h | 19 ++
> > 11 files changed, 226 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/drm_damage_helper.c
> > create mode 100644 include/drm/drm_damage_helper.h
> >
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-
> kms.rst
> > index 5dee6b8a4c12..78b66e628857 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -542,6 +542,15 @@ Plane Composition Properties
> > .. kernel-doc:: drivers/gpu/drm/drm_blend.c
> > :export:
> >
> > +FB_DAMAGE_CLIPS
> > +~~~
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
> > + :doc: overview
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
> > + :export:
> > +
> > Color Management Properties
> > ---
> >
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index a6771cef85e2..ca5be0decb3f 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -35,7 +35,7 @@ drm_kms_helper-y := drm_crtc_helper.o
> drm_dp_helper.o drm_probe_helper.o \
> > drm_plane_helper.o drm_dp_mst_topology.o
> drm_atomic_helper.o \
> > drm_kms_helper_common.o drm_dp_dual_mode_helper.o
> \
> > drm_simple_kms_helper.o drm_modeset_helper.o \
> > - drm_scdc_helper.o drm_gem_framebuffer_helper.o
> > + drm_scdc_helper.o drm_gem_framebuffer_helper.o
> drm_damage_helper.o
> >
> > drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
> > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) +=
> drm_fb_helper.o
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> b/drivers/gpu/drm/drm_atomic.c
> > index 3eb061e11e2e..652e78ca1f81 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> > {
> > struct drm_device *dev = plane->dev;
> > struct drm_mode_config *config = >mode_config;
> > + bool replaced = false;
> > + int ret;
> >
> > if (property == config->prop_fb_id) {
> > struct drm_framebuffer *fb =
> drm_framebuffer_lookup(dev, NULL, val);
> > @@ -908,6 +910,14 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> > state->color_encoding = val;
> > } else if (property == plane->color_range_property) {
> > state->color_range = val;
> > + } else if (property == config->prop_fb_damage_clips) {
> > + ret = drm_atomic_replace_property_blob_from_id(dev,
> > + >fb_damage_clips,
> > + val,
> > + -1,
> > + sizeof(struct drm_rect),
> > + );
> > + return ret;
> > } else if (plane->funcs->atomic_set_property) {
> > return plane->funcs->atomic_set_property(plane, state,
> > property, val);
> > @@ -976,6 +986,9 @@ drm_atomic_plane_get_property(struct drm_plane
> *plane,
> > *val = state->color_encoding;
> > } else if (property == plane->color_range_property) {
> > *val = state->color_range;
> > + } else if
[PATCH v2 1/4] bochs: use drm_fb_helper_set_suspend_unlocked in suspend/resume
The "initialized" member is going away. suspend/resume still works (even
if bochsfb_create is forced to fail).
Signed-off-by: Peter Wu
---
drivers/gpu/drm/bochs/bochs_drv.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c
b/drivers/gpu/drm/bochs/bochs_drv.c
index c61b40c72b62..0e79d9acf89e 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -107,11 +107,7 @@ static int bochs_pm_suspend(struct device *dev)
drm_kms_helper_poll_disable(drm_dev);
- if (bochs->fb.initialized) {
- console_lock();
- drm_fb_helper_set_suspend(>fb.helper, 1);
- console_unlock();
- }
+ drm_fb_helper_set_suspend_unlocked(>fb.helper, 1);
return 0;
}
@@ -124,11 +120,7 @@ static int bochs_pm_resume(struct device *dev)
drm_helper_resume_force_mode(drm_dev);
- if (bochs->fb.initialized) {
- console_lock();
- drm_fb_helper_set_suspend(>fb.helper, 0);
- console_unlock();
- }
+ drm_fb_helper_set_suspend_unlocked(>fb.helper, 0);
drm_kms_helper_poll_enable(drm_dev);
return 0;
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] drm/fb-helper: improve documentation and print warnings
Clarify the relation between drm_fb_helper_fbdev_setup/teardown. Clarify
requirements for the new generic fbdev emulation API and log some more
details in case the driver does something wrong. Fix related typos.
Cc: Noralf Trønnes
Signed-off-by: Peter Wu
---
drivers/gpu/drm/drm_fb_helper.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4b0dd20bccb8..7f92ff173986 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2821,7 +2821,9 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
* The caller must to provide a _fb_helper_funcs->fb_probe callback
* function.
*
- * See also: drm_fb_helper_initial_config()
+ * Use drm_fb_helper_fbdev_teardown() to destroy the fbdev.
+ *
+ * See also: drm_fb_helper_initial_config(), drm_fbdev_generic_setup().
*
* Returns:
* Zero on success or negative error code on failure.
@@ -3037,7 +3039,7 @@ static struct fb_deferred_io drm_fbdev_defio = {
* @fb_helper: fbdev helper structure
* @sizes: describes fbdev size and scanout surface size
*
- * This function uses the client API to crate a framebuffer backed by a dumb
buffer.
+ * This function uses the client API to create a framebuffer backed by a dumb
buffer.
*
* The _sys_ versions are used for _ops.fb_read, fb_write, fb_fillrect,
* fb_copyarea, fb_imageblit.
@@ -3165,8 +3167,10 @@ static int drm_fbdev_client_hotplug(struct
drm_client_dev *client)
if (dev->fb_helper)
return drm_fb_helper_hotplug_event(dev->fb_helper);
- if (!dev->mode_config.num_connector)
+ if (!dev->mode_config.num_connector) {
+ DRM_DEV_DEBUG(dev->dev, "No connectors found, will not create
framebuffer!\n");
return 0;
+ }
ret = drm_fb_helper_fbdev_setup(dev, fb_helper,
_fb_helper_generic_funcs,
fb_helper->preferred_bpp, 0);
@@ -3187,13 +3191,15 @@ static const struct drm_client_funcs
drm_fbdev_client_funcs = {
};
/**
- * drm_fb_helper_generic_fbdev_setup() - Setup generic fbdev emulation
+ * drm_fbdev_generic_setup() - Setup generic fbdev emulation
* @dev: DRM device
* @preferred_bpp: Preferred bits per pixel for the device.
* @dev->mode_config.preferred_depth is used if this is zero.
*
* This function sets up generic fbdev emulation for drivers that supports
- * dumb buffers with a virtual address and that can be mmap'ed.
+ * dumb buffers with a virtual address and that can be mmap'ed. If the driver
+ * does not support these functions, it could use drm_fb_helper_fbdev_setup().
+ * This function can only be used with devices created using
drm_dev_register().
*
* Restore, hotplug events and teardown are all taken care of. Drivers that do
* suspend/resume need to call drm_fb_helper_set_suspend_unlocked() themselves.
@@ -3206,6 +3212,8 @@ static const struct drm_client_funcs
drm_fbdev_client_funcs = {
* This function is safe to call even when there are no connectors present.
* Setup will be retried on the next hotplug event.
*
+ * The fbdev is destroyed by drm_dev_unregister().
+ *
* Returns:
* Zero on success or negative error code on failure.
*/
@@ -3214,6 +3222,8 @@ int drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp)
struct drm_fb_helper *fb_helper;
int ret;
+ WARN(dev->fb_helper, "fb_helper is already set!\n");
+
if (!drm_fbdev_emulation)
return 0;
@@ -3224,12 +3234,15 @@ int drm_fbdev_generic_setup(struct drm_device *dev,
unsigned int preferred_bpp)
ret = drm_client_new(dev, _helper->client, "fbdev",
_fbdev_client_funcs);
if (ret) {
kfree(fb_helper);
+ DRM_DEV_ERROR(dev->dev, "Failed to register client: %d\n", ret);
return ret;
}
fb_helper->preferred_bpp = preferred_bpp;
- drm_fbdev_client_hotplug(_helper->client);
+ ret = drm_fbdev_client_hotplug(_helper->client);
+ if (ret)
+ DRM_DEV_DEBUG(dev->dev, "client hotplug ret=%d\n", ret);
return 0;
}
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] bochs: convert to drm_dev_register
The drm_get_pci_dev API is deprecated, replace it by drm_dev_register.
Signed-off-by: Peter Wu
---
drivers/gpu/drm/bochs/bochs.h | 2 +-
drivers/gpu/drm/bochs/bochs_drv.c | 34 +--
drivers/gpu/drm/bochs/bochs_hw.c | 2 +-
3 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 8514a84fbdbe..b4f6bb521900 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -117,7 +117,7 @@ static inline u64 bochs_bo_mmap_offset(struct bochs_bo *bo)
/* -- */
/* bochs_hw.c */
-int bochs_hw_init(struct drm_device *dev, uint32_t flags);
+int bochs_hw_init(struct drm_device *dev);
void bochs_hw_fini(struct drm_device *dev);
void bochs_hw_setmode(struct bochs_device *bochs,
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c
b/drivers/gpu/drm/bochs/bochs_drv.c
index 0e79d9acf89e..f3dd66ae990a 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -35,7 +35,7 @@ static void bochs_unload(struct drm_device *dev)
dev->dev_private = NULL;
}
-static int bochs_load(struct drm_device *dev, unsigned long flags)
+static int bochs_load(struct drm_device *dev)
{
struct bochs_device *bochs;
int ret;
@@ -46,7 +46,7 @@ static int bochs_load(struct drm_device *dev, unsigned long
flags)
dev->dev_private = bochs;
bochs->dev = dev;
- ret = bochs_hw_init(dev, flags);
+ ret = bochs_hw_init(dev);
if (ret)
goto err;
@@ -82,8 +82,6 @@ static const struct file_operations bochs_fops = {
static struct drm_driver bochs_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET,
- .load = bochs_load,
- .unload = bochs_unload,
.fops = _fops,
.name = "bochs-drm",
.desc = "bochs dispi vga interface (qemu stdvga)",
@@ -138,6 +136,7 @@ static const struct dev_pm_ops bochs_pm_ops = {
static int bochs_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
+ struct drm_device *dev;
unsigned long fbsize;
int ret;
@@ -151,14 +150,37 @@ static int bochs_pci_probe(struct pci_dev *pdev,
if (ret)
return ret;
- return drm_get_pci_dev(pdev, ent, _driver);
+ dev = drm_dev_alloc(_driver, >dev);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+
+ dev->pdev = pdev;
+ pci_set_drvdata(pdev, dev);
+
+ ret = bochs_load(dev);
+ if (ret)
+ goto err_free_dev;
+
+ ret = drm_dev_register(dev, 0);
+ if (ret)
+ goto err_unload;
+
+ return ret;
+
+err_unload:
+ bochs_unload(dev);
+err_free_dev:
+ drm_dev_put(dev);
+ return ret;
}
static void bochs_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- drm_put_dev(dev);
+ drm_dev_unregister(dev);
+ bochs_unload(dev);
+ drm_dev_put(dev);
}
static const struct pci_device_id bochs_pci_tbl[] = {
diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
index a39b0343c197..16e4f1caccca 100644
--- a/drivers/gpu/drm/bochs/bochs_hw.c
+++ b/drivers/gpu/drm/bochs/bochs_hw.c
@@ -47,7 +47,7 @@ static void bochs_dispi_write(struct bochs_device *bochs, u16
reg, u16 val)
}
}
-int bochs_hw_init(struct drm_device *dev, uint32_t flags)
+int bochs_hw_init(struct drm_device *dev)
{
struct bochs_device *bochs = dev->dev_private;
struct pci_dev *pdev = dev->pdev;
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/4] bochs: convert to drm_fb_helper_fbdev_setup/teardown
Currently unloading bochs_drm (after unbinding the vtconsole) results in
a warning about a leaked connector:
[drm:drm_mode_config_cleanup] *ERROR* connector Virtual-3 leaked!
While investigating a potential fix I noticed that a lot of open-coded
functionality is already implemented elsewhere, so start converting it:
bochs_fbdev_init -> drm_fb_helper_fbdev_setup: trivial (similar impl).
bochs_fbdev_fini -> drm_fb_helper_fbdev_teardown: requires unembedding
"struct drm_framebuffer" from "struct bochs_framebuffer".
Unembedding drm_framebuffer is made easy using drm_gem_fbdev_fb_create
which can replace bochs_fbdev_destroy and custom routines in bochs_mm.c.
For this to work, the GEM object is moved into "drm_framebuffer". After
that, "bochs_framebuffer" is no longer needed and therefore removed.
Remove the unused "size" and "initialized" fields from fb, the latter is
not necessary as drm_fb_helper_fbdev_teardown can be called even if
bochsfb_create fails. This theory was tested by returning early and
late (just before drm_gem_fbdev_fb_create). Both scenarios fail
gracefully although the latter seems to leak the object from
bochsfb_create_object (not a regression).
Guess on the reason for the encoder leak: drm_framebuffer_cleanup was
previously used, but did not destroy much. drm_fb_helper_fbdev_teardown
is now used and calls drm_framebuffer_remove which does a bit more work.
Tested with 'echo 0 > /sys/class/vtconsole/vtcon1/bind; rmmod bochs_drm'
and also with Xorg + fbdev (startx -> xterm). The latter triggered a
warning in ttm_bo_vm_open that existed before, see
https://lkml.kernel.org/r/1464000533-13140-4-git-send-email-msta...@suse.de
Acked-by: Daniel Vetter
Signed-off-by: Peter Wu
---
drivers/gpu/drm/bochs/bochs.h | 19 ++-
drivers/gpu/drm/bochs/bochs_fbdev.c | 79 +++--
drivers/gpu/drm/bochs/bochs_kms.c | 7 +--
drivers/gpu/drm/bochs/bochs_mm.c| 74 ---
4 files changed, 22 insertions(+), 157 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 375bf92cd04f..8514a84fbdbe 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -51,11 +51,6 @@ enum bochs_types {
BOCHS_UNKNOWN,
};
-struct bochs_framebuffer {
- struct drm_framebuffer base;
- struct drm_gem_object *obj;
-};
-
struct bochs_device {
/* hw */
void __iomem *mmio;
@@ -88,15 +83,11 @@ struct bochs_device {
/* fbdev */
struct {
- struct bochs_framebuffer gfb;
+ struct drm_framebuffer *fb;
struct drm_fb_helper helper;
- int size;
- bool initialized;
} fb;
};
-#define to_bochs_framebuffer(x) container_of(x, struct bochs_framebuffer, base)
-
struct bochs_bo {
struct ttm_buffer_object bo;
struct ttm_placement placement;
@@ -148,15 +139,9 @@ int bochs_dumb_create(struct drm_file *file, struct
drm_device *dev,
int bochs_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev,
uint32_t handle, uint64_t *offset);
-int bochs_framebuffer_init(struct drm_device *dev,
- struct bochs_framebuffer *gfb,
- const struct drm_mode_fb_cmd2 *mode_cmd,
- struct drm_gem_object *obj);
int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag, u64 *gpu_addr);
int bochs_bo_unpin(struct bochs_bo *bo);
-extern const struct drm_mode_config_funcs bochs_mode_funcs;
-
/* bochs_kms.c */
int bochs_kms_init(struct bochs_device *bochs);
void bochs_kms_fini(struct bochs_device *bochs);
@@ -164,3 +149,5 @@ void bochs_kms_fini(struct bochs_device *bochs);
/* bochs_fbdev.c */
int bochs_fbdev_init(struct bochs_device *bochs);
void bochs_fbdev_fini(struct bochs_device *bochs);
+
+extern const struct drm_mode_config_funcs bochs_mode_funcs;
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 14eb8d0d5a00..8f4d6c052f7b 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -6,6 +6,7 @@
*/
#include "bochs.h"
+#include
/* -- */
@@ -13,9 +14,7 @@ static int bochsfb_mmap(struct fb_info *info,
struct vm_area_struct *vma)
{
struct drm_fb_helper *fb_helper = info->par;
- struct bochs_device *bochs =
- container_of(fb_helper, struct bochs_device, fb.helper);
- struct bochs_bo *bo = gem_to_bochs_bo(bochs->fb.gfb.obj);
+ struct bochs_bo *bo = gem_to_bochs_bo(fb_helper->fb->obj[0]);
return ttm_fbdev_mmap(vma, >bo);
}
@@ -101,19 +100,20 @@ static int bochsfb_create(struct drm_fb_helper *helper,
/* init fb device */
info = drm_fb_helper_alloc_fbi(helper);
- if (IS_ERR(info))
+ if (IS_ERR(info)) {
+ DRM_ERROR("Failed
[PATCH v2 0/4] bochs fixes and fb-helper documentation updates
Hi,
These series tries to improve the bochs driver and update documentation based on
my brief experience with the fb-helper API. Thank you Daniel and Gerd for your
previous feedback and helpful suggestions.
Patch 2 was previously posted and is unmodified except for acked-by + rebase on
drm-misc-next (9a09a42369a4a37a959c051d8e1a1f948c1529a4). Patch 1 is a trivial
(build) fix that was missing last time.
Patch 3 converts from the legacy API to the modern drm_dev_register approach.
This seems required for the "generic" fbdev API as suggested by Daniel, but as
bochs does not implement the required "gem_prime_vmap" function, the conversion
cannot be completed for now.
Patch 4 includes some documentation updates that would have helped me during
qxl/bochs development and a warning that made me realize that "a virtual address
and that can be mmap'ed" in the documentation referred to "gem_prime_vmap". It
is to my best of understanding, so please correct me if I am wrong.
Side note: I originally tried to fix the unbind/remove crash in QXL and then
turned to bochs as it seemed simpler to learn how to work on DRM drivers.
Hopefully I manage to eventually figure out how to fix QXL. QXL is a bit
strange, it advertises PRIME "support" but it only has stub functions (including
a stub gem_prime_vmap).
After my recent patch ("qxl: fix null-pointer crash during suspend") qxl can
suspend/resume in the normal situation, but doing anything (suspend or unload)
after unbinding just fails (suspend then fails with "failed to wait on release
23 after spincount 301", unload triggers a use-after-free according to KASAN).
On the other hand, bochs passes these tests:
# 1. s/r with unbound console
modprobe bochs_drm
echo 0 > /sys/class/vtconsole/vtcon1/bind
rtcwake -s 1 -m mem
# 2. s/r in normal sitation
echo 1 > /sys/class/vtconsole/vtcon1/bind
rtcwake -s 1 -m mem
# 3. unload module (and s/r for good measure)
echo 0 > /sys/class/vtconsole/vtcon1/bind
rmmod bochs_drm
rtcwake -s 1 -m mem
Kind regards,
Peter
Peter Wu (4):
bochs: use drm_fb_helper_set_suspend_unlocked in suspend/resume
bochs: convert to drm_fb_helper_fbdev_setup/teardown
bochs: convert to drm_dev_register
drm/fb-helper: improve documentation and print warnings
drivers/gpu/drm/bochs/bochs.h | 21 ++--
drivers/gpu/drm/bochs/bochs_drv.c | 46 +++--
drivers/gpu/drm/bochs/bochs_fbdev.c | 79 +++--
drivers/gpu/drm/bochs/bochs_hw.c| 2 +-
drivers/gpu/drm/bochs/bochs_kms.c | 7 +--
drivers/gpu/drm/bochs/bochs_mm.c| 74 ---
drivers/gpu/drm/drm_fb_helper.c | 25 ++---
7 files changed, 73 insertions(+), 181 deletions(-)
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/14] drm/amdgpu: Remove default best_encoder hook from DC
On Thu, Sep 6, 2018 at 6:33 PM, Deucher, Alexander
wrote:
>> -Original Message-
>> From: Daniel Vetter
>> Sent: Wednesday, September 5, 2018 9:48 AM
>> To: Li, Sun peng (Leo)
>> Cc: DRI Development ; Intel Graphics
>> Development ; Daniel Vetter
>> ; Deucher, Alexander
>> ; Wentland, Harry
>> ; Grodzovsky, Andrey
>> ; Cheng, Tony ; S,
>> Shirish
>> Subject: Re: [PATCH 08/14] drm/amdgpu: Remove default best_encoder
>> hook from DC
>>
>> On Wed, Sep 5, 2018 at 3:45 PM, Leo Li wrote:
>> >
>> >
>> > On 2018-09-03 12:54 PM, Daniel Vetter wrote:
>> >>
>> >> For atomic driver this is the default, no need to reimplement it. We
>> >> still need to keep the copypasta for not-atomic drivers though, since
>> >> no one polished the legacy crtc helpers as much as the atomic ones.
>> >
>> >
>> > Thanks for the patch! It seems I made an identical change here:
>> > https://lists.freedesktop.org/archives/amd-gfx/2018-August/026064.html
>> >
>> > One line difference though. I wasn't aware that the default is used
>> > when .best_encoder is null, so I just hooked it to the default helper.
>> >
>> > If it's OK with you, I'll do a follow-up to drop the hook, so we don't
>> > have two conflicting patches in flight.
>>
>> I have a follow-up patches to unexport the helper, so would be good if Alex
>> sends out the pull request so I can sort this all out in drm-misc-next. I
>> don't
>> want to split up the patch series if possible.
>> Or we just deal with the conflict, it's minor.
>>
>> Alex?
>
> I'm planning to send the PR next week when I'm back in the office. I can
> drop Leo's patch when I send the PR if that makes things easier.
I'm happy to rebase, just go ahead. I've added a bunch more patches to
my helper cleanup series anyway, so will take a bit longer to get all
landed.
-Daniel
>
> Alex
>
>> -Daniel
>>
>> > Leo
>> >
>> >
>> >>
>> >> Signed-off-by: Daniel Vetter
>> >> Cc: Alex Deucher
>> >> Cc: Harry Wentland
>> >> Cc: Andrey Grodzovsky
>> >> Cc: Tony Cheng
>> >> Cc: "Leo (Sunpeng) Li"
>> >> Cc: Shirish S
>> >> ---
>> >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23
>> ---
>> >> 1 file changed, 23 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> index af6adffba788..333f9904f135 100644
>> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> @@ -2794,28 +2794,6 @@ static const struct drm_connector_funcs
>> >> amdgpu_dm_connector_funcs = {
>> >> .atomic_get_property =
>> amdgpu_dm_connector_atomic_get_property
>> >> };
>> >> -static struct drm_encoder *best_encoder(struct drm_connector
>> >> *connector)
>> >> -{
>> >> - int enc_id = connector->encoder_ids[0];
>> >> - struct drm_mode_object *obj;
>> >> - struct drm_encoder *encoder;
>> >> -
>> >> - DRM_DEBUG_DRIVER("Finding the best encoder\n");
>> >> -
>> >> - /* pick the encoder ids */
>> >> - if (enc_id) {
>> >> - obj = drm_mode_object_find(connector->dev, NULL, enc_id,
>> >> DRM_MODE_OBJECT_ENCODER);
>> >> - if (!obj) {
>> >> - DRM_ERROR("Couldn't find a matching encoder for
>> >> our connector\n");
>> >> - return NULL;
>> >> - }
>> >> - encoder = obj_to_encoder(obj);
>> >> - return encoder;
>> >> - }
>> >> - DRM_ERROR("No encoder id\n");
>> >> - return NULL;
>> >> -}
>> >> -
>> >> static int get_modes(struct drm_connector *connector)
>> >> {
>> >> return amdgpu_dm_connector_get_modes(connector);
>> >> @@ -2934,7 +2912,6 @@ amdgpu_dm_connector_helper_funcs = {
>> >> */
>> >> .get_modes = get_modes,
>> >> .mode_valid = amdgpu_dm_connector_mode_valid,
>> >> - .best_encoder = best_encoder
>> >> };
>> >> static void dm_crtc_helper_disable(struct drm_crtc *crtc)
>> >>
>> >
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 03/14] drm: clear plane damage during full modeset
> > drivers/gpu/drm/drm_atomic_helper.c | 4
> > include/drm/drm_damage_helper.h | 10 ++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> > index be83e2763c18..e06d2d5d582f 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -31,6 +31,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include "drm_crtc_helper_internal.h"
> > @@ -88,6 +89,9 @@ drm_atomic_helper_plane_changed(struct
> drm_atomic_state *state,
> > return;
> >
> > crtc_state->planes_changed = true;
> > +
> > + if (drm_atomic_crtc_needs_modeset(crtc_state))
> > + drm_plane_clear_damage(plane_state);
>
> I'm not 100% sure this is the best place to put this. I'm also wondering
> whether we should clear damage when moving planes between crtc on top
> of
> this. But I guess vmwgfx doesn't allow that, we can figure this out when
> the first driver with moveable planes adds damage support.
Yes I agree this is not the best place but it was the one with minimal code
change. IMO should have a separate function for clearing damage.
> > }
> > }
> >
> > diff --git a/include/drm/drm_damage_helper.h
> b/include/drm/drm_damage_helper.h
> > index f1282b459a4f..1f988f7fdd72 100644
> > --- a/include/drm/drm_damage_helper.h
> > +++ b/include/drm/drm_damage_helper.h
> > @@ -71,6 +71,16 @@ drm_plane_get_damage_clips(const struct
> drm_plane_state *state)
> >state->fb_damage_clips->data : NULL);
> > }
> >
> > +/**
> > + * drm_plane_clear_damage - clears damage blob in a plane state
> > + * @state: Plane state
>
> A bit more kerneldoc would be good. Maybe explain how that impacts the
> damage iterator - you get full damage after calling this, which is a bit
> confusing for a function called clear_damage. So definitely worth
> explaining.
>
> With the kerneldoc beefed up:
Agreed.
>
> Reviewed-by: Daniel Vetter
>
> > + */
> > +static inline void drm_plane_clear_damage(struct drm_plane_state
> *state)
> > +{
> > + drm_property_blob_put(state->fb_damage_clips);
> > + state->fb_damage_clips = NULL;
> > +}
> > +
> > void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
> > int
> > drm_atomic_helper_damage_iter_init(struct
> drm_atomic_helper_damage_iter *iter,
> > --
> > 2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 02/14] drm: add helper iterator functions for plane fb_damage_clips blob
> > #include
> >
> > /**
> > @@ -75,6 +76,11 @@
> > * While kernel does not error for overlapped damage clips, it is
> discouraged.
> > */
> >
> > +static int convert_fixed_to_32(int fixed)
> > +{
> > + return ((fixed >> 15) & 1) + (fixed >> 16);
> > +}
> > +
> > /**
> > * drm_plane_enable_fb_damage_clips - enables plane fb damage clips
> property
> > * @plane: plane on which to enable damage clips property
> > @@ -90,3 +96,90 @@ void drm_plane_enable_fb_damage_clips(struct
> drm_plane *plane)
> >0);
> > }
> > EXPORT_SYMBOL(drm_plane_enable_fb_damage_clips);
> > +
> > +/**
> > + * drm_atomic_helper_damage_iter_init - initialize the damage iterator
> > + * @iter: The iterator to initialize.
> > + * @old_state: old plane state for validation.
> > + * @new_state: plane state from which to iterate the damage clips.
> > + *
> > + * Initialize an iterator that clip framebuffer damage in plane
> fb_damage_clips
> > + * blob to plane src clip. The iterator returns full plane src in case
> > needing
> > + * full update e.g. during full modeset.
> > + *
> > + * With this helper iterator, drivers which enabled fb_damage_clips
> property can
> > + * iterate over the damage clips that falls inside plane src during plane
> > + * update.
> > + *
> > + * Returns: 0 on success and negative error code on error. If an error code
> is
> > + * returned then it means the plane state shouldn't update with attached
> fb.
> > + */
> > +int
> > +drm_atomic_helper_damage_iter_init(struct
> drm_atomic_helper_damage_iter *iter,
> > + const struct drm_plane_state *old_state,
> > + const struct drm_plane_state *new_state)
> > +{
> > + if (!new_state || !new_state->crtc || !new_state->fb)
> > + return -EINVAL;
> > +
> > + if (!new_state->visible)
> > + return -EINVAL;
>
> Can't we handle this by iterating 0 damage rects instead? Would make the
> code a bit cleaner I think.
Agreed.
>
> > +
> > + memset(iter, 0, sizeof(*iter));
> > + iter->clips = drm_plane_get_damage_clips(new_state);
> > + iter->num_clips = drm_plane_get_damage_clips_count(new_state);
> > +
> > + if (!iter->clips)
> > + iter->full_update = true;
> > +
> > + if (!drm_rect_equals(_state->src, _state->src))
> > + iter->full_update = true;
> > +
> > + iter->plane_src.x1 = convert_fixed_to_32(new_state->src.x1);
> > + iter->plane_src.y1 = convert_fixed_to_32(new_state->src.y1);
> > + iter->plane_src.x2 = convert_fixed_to_32(new_state->src.x2);
> > + iter->plane_src.y2 = convert_fixed_to_32(new_state->src.y2);
>
> I think you want to clip with the clipped rectangles here, not with the
> ones userspace provides.
new_state->src.x1 is the clipped one, clipped in the function
drm_atomic_helper_check_plane_state. Or am I missing something ?
>
> Also I think you're rounding is wrong here - I think you need to round
> down for x/y1, and round up for x/y2 to make sure you catch all the
> pixels?
>
> Unit tests for this, in the form of a drm selftest (like we have for
> drm_mm.c already, but for kms helpers) would be perfect. Much easier to
> review a testcase than do bitmath in my head :-)
Sure I will work on a unit test case for this scenario and work on round up
of plane_src.
>
> > +
> > + if (iter->full_update) {
> > + iter->clips = 0;
> > + iter->curr_clip = 0;
> > + iter->num_clips = 0;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_damage_iter_init);
> > +
> > +/**
> > + * drm_atomic_helper_damage_iter_next - advance the damage iterator
> > + * @iter: The iterator to advance.
> > + * @rect: Return a rectangle in fb coordinate clipped to plane src.
> > + *
> > + * Returns: true if the output is valid, false if we've reached the end of
> the
> > + * rectangle list.
> > + */
> > +bool
> > +drm_atomic_helper_damage_iter_next(struct
> drm_atomic_helper_damage_iter *iter,
> > + struct drm_rect *rect)
> > +{
> > + bool ret = false;
> > +
> > + if (iter->full_update) {
> > + *rect = iter->plane_src;
> > + iter->full_update = false;
> > + return true;
> > + }
> > +
> > + while (iter->curr_clip < iter->num_clips) {
> > + *rect = iter->clips[iter->curr_clip];
> > + iter->curr_clip++;
> > +
> > + if (drm_rect_intersect(rect, >plane_src)) {
> > + ret = true;
> > + break;
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
> > diff --git a/include/drm/drm_damage_helper.h
> b/include/drm/drm_damage_helper.h
> > index 217694e9168c..f1282b459a4f 100644
> > --- a/include/drm/drm_damage_helper.h
> > +++ b/include/drm/drm_damage_helper.h
> > @@ -32,6 +32,19 @@
> > #ifndef DRM_DAMAGE_HELPER_H_
> > #define DRM_DAMAGE_HELPER_H_
> >
> > +/**
> > + * struct
[PATCH v4 2/6] drm/nouveau: Add NV_PRINTK_ONCE and variants
Since we're about to use this in nouveau_backlight.c. Same thing as
DRM_WARN_ONCE, DRM_INFO_ONCE, etc...
Signed-off-by: Lyude Paul
Cc: Karol Herbst
---
drivers/gpu/drm/nouveau/nouveau_drv.h | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 6e1acaec3400..d9d687916553 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -244,10 +244,12 @@ void nouveau_drm_device_remove(struct drm_device *dev);
struct nouveau_cli *_cli = (c);\
dev_##l(_cli->drm->dev->dev, "%s: "f, _cli->name, ##a);\
} while(0)
+
#define NV_FATAL(drm,f,a...) NV_PRINTK(crit, &(drm)->client, f, ##a)
#define NV_ERROR(drm,f,a...) NV_PRINTK(err, &(drm)->client, f, ##a)
#define NV_WARN(drm,f,a...) NV_PRINTK(warn, &(drm)->client, f, ##a)
#define NV_INFO(drm,f,a...) NV_PRINTK(info, &(drm)->client, f, ##a)
+
#define NV_DEBUG(drm,f,a...) do {
\
if (unlikely(drm_debug & DRM_UT_DRIVER)) \
NV_PRINTK(info, &(drm)->client, f, ##a); \
@@ -257,6 +259,12 @@ void nouveau_drm_device_remove(struct drm_device *dev);
NV_PRINTK(info, &(drm)->client, f, ##a); \
} while(0)
+#define NV_PRINTK_ONCE(l,c,f,a...) NV_PRINTK(l##_once,c,f, ##a)
+
+#define NV_ERROR_ONCE(drm,f,a...) NV_PRINTK_ONCE(err, &(drm)->client, f, ##a)
+#define NV_WARN_ONCE(drm,f,a...) NV_PRINTK_ONCE(warn, &(drm)->client, f, ##a)
+#define NV_INFO_ONCE(drm,f,a...) NV_PRINTK_ONCE(info, &(drm)->client, f, ##a)
+
extern int nouveau_modeset;
#endif
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 3/6] drm/nouveau: Move backlight device into nouveau_connector
Currently module unloading is broken in nouveau due to a rather annoying race condition resulting from nouveau_backlight.c having gone a bit stale over time: [ 1960.791143] == [ 1960.791394] BUG: KASAN: use-after-free in nouveau_backlight_exit+0x112/0x150 [nouveau] [ 1960.791460] Read of size 4 at addr 88075accf350 by task zsh/11185 [ 1960.791521] [ 1960.791545] CPU: 7 PID: 11185 Comm: zsh Kdump: loaded Tainted: G O 4.18.0Lyude-Test+ #4 [ 1960.791580] Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET79W (1.52 ) 07/13/2018 [ 1960.791628] Call Trace: [ 1960.791680] dump_stack+0xa4/0xfd [ 1960.791721] print_address_description+0x71/0x239 [ 1960.791833] ? nouveau_backlight_exit+0x112/0x150 [nouveau] [ 1960.791877] kasan_report.cold.6+0x242/0x2fe [ 1960.791919] __asan_report_load4_noabort+0x19/0x20 [ 1960.792012] nouveau_backlight_exit+0x112/0x150 [nouveau] [ 1960.792081] nouveau_display_destroy+0x76/0x150 [nouveau] [ 1960.792150] nouveau_drm_device_fini+0xb7/0x190 [nouveau] [ 1960.792265] nouveau_drm_device_remove+0x14b/0x1d0 [nouveau] [ 1960.792347] ? nouveau_cli_work_queue+0x2e0/0x2e0 [nouveau] [ 1960.792378] ? trace_hardirqs_on_caller+0x38b/0x570 [ 1960.792406] ? trace_hardirqs_on+0xd/0x10 [ 1960.792472] nouveau_drm_remove+0x37/0x50 [nouveau] [ 1960.792502] pci_device_remove+0x112/0x2d0 [ 1960.792530] ? pcibios_free_irq+0x10/0x10 [ 1960.792558] ? kasan_check_write+0x14/0x20 [ 1960.792587] device_release_driver_internal+0x35c/0x650 [ 1960.792617] device_release_driver+0x12/0x20 [ 1960.792643] pci_stop_bus_device+0x172/0x1e0 [ 1960.792671] pci_stop_and_remove_bus_device_locked+0x1a/0x30 [ 1960.792715] remove_store+0xcb/0xe0 [ 1960.792753] ? sriov_numvfs_store+0x2e0/0x2e0 [ 1960.792779] ? __lock_is_held+0xb5/0x140 [ 1960.792808] ? component_add+0x530/0x530 [ 1960.792834] dev_attr_store+0x3f/0x70 [ 1960.792859] ? sysfs_file_ops+0x11d/0x170 [ 1960.792885] sysfs_kf_write+0x104/0x150 [ 1960.792915] ? sysfs_file_ops+0x170/0x170 [ 1960.792940] kernfs_fop_write+0x24f/0x400 [ 1960.792978] ? __lock_acquire+0x6ea/0x47f0 [ 1960.793021] __vfs_write+0xeb/0x760 [ 1960.793048] ? kernel_read+0x130/0x130 [ 1960.793076] ? __lock_is_held+0xb5/0x140 [ 1960.793107] ? rcu_read_lock_sched_held+0xdd/0x110 [ 1960.793135] ? rcu_sync_lockdep_assert+0x78/0xb0 [ 1960.793162] ? __sb_start_write+0x183/0x220 [ 1960.793189] vfs_write+0x14d/0x4a0 [ 1960.793229] ksys_write+0xd2/0x1b0 [ 1960.793255] ? __ia32_sys_read+0xb0/0xb0 [ 1960.793298] ? fput+0x1d/0x120 [ 1960.793324] ? filp_close+0xf3/0x130 [ 1960.793349] ? entry_SYSCALL_64_after_hwframe+0x59/0xbe [ 1960.793380] __x64_sys_write+0x73/0xb0 [ 1960.793407] do_syscall_64+0xaa/0x400 [ 1960.793433] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 1960.793460] RIP: 0033:0x7f59df433164 [ 1960.793486] Code: 89 02 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 8d 05 81 38 2d 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 49 89 d4 55 48 89 f5 53 [ 1960.793541] RSP: 002b:7ffd70ee2fb8 EFLAGS: 0246 ORIG_RAX: 0001 [ 1960.793576] RAX: ffda RBX: 0002 RCX: 7f59df433164 [ 1960.793620] RDX: 0002 RSI: 5578088640c0 RDI: 0001 [ 1960.793665] RBP: 5578088640c0 R08: 7f59df7038c0 R09: 7f59e0995b80 [ 1960.793696] R10: 000a R11: 0246 R12: 7f59df702760 [ 1960.793730] R13: 0002 R14: 7f59df6fd760 R15: 0002 [ 1960.793768] [ 1960.793790] Allocated by task 11167: [ 1960.793816] save_stack+0x43/0xd0 [ 1960.793841] kasan_kmalloc+0xc4/0xe0 [ 1960.793880] kasan_slab_alloc+0x11/0x20 [ 1960.793905] kmem_cache_alloc+0xd7/0x270 [ 1960.793944] getname_flags+0xbd/0x520 [ 1960.793969] user_path_at_empty+0x23/0x50 [ 1960.793994] do_faccessat+0x1fc/0x5d0 [ 1960.794018] __x64_sys_access+0x59/0x80 [ 1960.794043] do_syscall_64+0xaa/0x400 [ 1960.794067] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 1960.794093] [ 1960.794127] Freed by task 11167: [ 1960.794152] save_stack+0x43/0xd0 [ 1960.794190] __kasan_slab_free+0x139/0x190 [ 1960.794215] kasan_slab_free+0xe/0x10 [ 1960.794239] kmem_cache_free+0xcb/0x2c0 [ 1960.794264] putname+0xad/0xe0 [ 1960.794287] filename_lookup.part.59+0x1f1/0x360 [ 1960.794313] user_path_at_empty+0x3e/0x50 [ 1960.794338] do_faccessat+0x1fc/0x5d0 [ 1960.794362] __x64_sys_access+0x59/0x80 [ 1960.794393] do_syscall_64+0xaa/0x400 [ 1960.794421] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 1960.794461] [ 1960.794483] The buggy address belongs to the object at 88075acceac0 [ 1960.794483] which belongs to the cache names_cache of size 4096 [ 1960.794540] The buggy address is located 2192 bytes inside of [ 1960.794540] 4096-byte region [88075acceac0, 88075accfac0) [ 1960.794581] The buggy address belongs to the page: [ 1960.794609] page:ea001d6b3200 count:1
[PATCH v4 1/6] drm/nouveau: Check backlight IDs are >= 0, not > 0
Remember, ida IDs start at 0, not 1!
Signed-off-by: Lyude Paul
Reviewed-by: Karol Herbst
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/nouveau/nouveau_backlight.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 408b955e5c39..6dd72bc32897 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -116,7 +116,7 @@ nv40_backlight_init(struct drm_connector *connector)
_bl_ops, );
if (IS_ERR(bd)) {
- if (bl_connector.id > 0)
+ if (bl_connector.id >= 0)
ida_simple_remove(_ida, bl_connector.id);
return PTR_ERR(bd);
}
@@ -249,7 +249,7 @@ nv50_backlight_init(struct drm_connector *connector)
nv_encoder, ops, );
if (IS_ERR(bd)) {
- if (bl_connector.id > 0)
+ if (bl_connector.id >= 0)
ida_simple_remove(_ida, bl_connector.id);
return PTR_ERR(bd);
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 5/6] drm/nouveau: Cleanup indenting in nouveau_backlight.c
Still no functional changes.
Signed-off-by: Lyude Paul
Reviewed-by: Karol Herbst
---
drivers/gpu/drm/nouveau/nouveau_backlight.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index f4400a6408b4..01d08acac2f0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -68,7 +68,7 @@ nv40_get_intensity(struct backlight_device *bd)
struct nouveau_drm *drm = bl_get_data(bd);
struct nvif_object *device = >client.device.object;
int val = (nvif_rd32(device, NV40_PMC_BACKLIGHT) &
- NV40_PMC_BACKLIGHT_MASK) >> 16;
+ NV40_PMC_BACKLIGHT_MASK) >> 16;
return val;
}
@@ -82,7 +82,7 @@ nv40_set_intensity(struct backlight_device *bd)
int reg = nvif_rd32(device, NV40_PMC_BACKLIGHT);
nvif_wr32(device, NV40_PMC_BACKLIGHT,
-(val << 16) | (reg & ~NV40_PMC_BACKLIGHT_MASK));
+ (val << 16) | (reg & ~NV40_PMC_BACKLIGHT_MASK));
return 0;
}
@@ -164,7 +164,7 @@ nv50_set_intensity(struct backlight_device *bd)
u32 val = (bd->props.brightness * div) / 100;
nvif_wr32(device, NV50_PDISP_SOR_PWM_CTL(or),
- NV50_PDISP_SOR_PWM_CTL_NEW | val);
+ NV50_PDISP_SOR_PWM_CTL_NEW | val);
return 0;
}
@@ -204,9 +204,10 @@ nva3_set_intensity(struct backlight_device *bd)
div = nvif_rd32(device, NV50_PDISP_SOR_PWM_DIV(or));
val = (bd->props.brightness * div) / 100;
if (div) {
- nvif_wr32(device, NV50_PDISP_SOR_PWM_CTL(or), val |
- NV50_PDISP_SOR_PWM_CTL_NEW |
- NVA3_PDISP_SOR_PWM_CTL_UNK);
+ nvif_wr32(device, NV50_PDISP_SOR_PWM_CTL(or),
+ val |
+ NV50_PDISP_SOR_PWM_CTL_NEW |
+ NVA3_PDISP_SOR_PWM_CTL_UNK);
return 0;
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 0/6] Backlight fixes and cleanup
Refactor for Ben, hopefully this time this should apply to his tree. Next version of https://patchwork.freedesktop.org/series/48596/ No changes otherwise. Lyude Paul (6): drm/nouveau: Check backlight IDs are >= 0, not > 0 drm/nouveau: Add NV_PRINTK_ONCE and variants drm/nouveau: Move backlight device into nouveau_connector drm/nouveau: s/nouveau_backlight_exit/nouveau_backlight_fini/ drm/nouveau: Cleanup indenting in nouveau_backlight.c drm/nouveau: Refactor nvXX_backlight_init() drivers/gpu/drm/nouveau/nouveau_backlight.c | 220 ++-- drivers/gpu/drm/nouveau/nouveau_connector.c | 20 ++ drivers/gpu/drm/nouveau/nouveau_connector.h | 33 +++ drivers/gpu/drm/nouveau/nouveau_display.c | 2 - drivers/gpu/drm/nouveau/nouveau_display.h | 25 --- drivers/gpu/drm/nouveau/nouveau_drv.h | 10 +- 6 files changed, 168 insertions(+), 142 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 6/6] drm/nouveau: Refactor nvXX_backlight_init()
There's literally no difference between any of the backlight init
functions besides the backlight properties they set and the backlight
callbacks that they set, so move all of the duplicated backlight init
code out of there and into nouveau_backlight_init().
This gets rid of a lot of copy pasta!
Changes since v1:
- Some of the pre-refactor callbacks were storing nv_encoder in callback
data for the backlight devices that they registered, as opposed to
nouveau_drm. This got missed and caused some bugs that didn't
originally appear on my setup (NULL kernel derefs) for some reason.
So, fix this by finding the nouveau_encoder in
nouveau_backlight_init(), and using that as the callback data for all
gens instead even if they don't care about the encoder.
Signed-off-by: Lyude Paul
Cc: Jeffery Miller
Cc: Karol Herbst
---
drivers/gpu/drm/nouveau/nouveau_backlight.c | 160 +---
1 file changed, 72 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 01d08acac2f0..5f5be6368aed 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -65,7 +65,8 @@ nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE],
static int
nv40_get_intensity(struct backlight_device *bd)
{
- struct nouveau_drm *drm = bl_get_data(bd);
+ struct nouveau_encoder *nv_encoder = bl_get_data(bd);
+ struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
int val = (nvif_rd32(device, NV40_PMC_BACKLIGHT) &
NV40_PMC_BACKLIGHT_MASK) >> 16;
@@ -76,7 +77,8 @@ nv40_get_intensity(struct backlight_device *bd)
static int
nv40_set_intensity(struct backlight_device *bd)
{
- struct nouveau_drm *drm = bl_get_data(bd);
+ struct nouveau_encoder *nv_encoder = bl_get_data(bd);
+ struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
int val = bd->props.brightness;
int reg = nvif_rd32(device, NV40_PMC_BACKLIGHT);
@@ -94,48 +96,20 @@ static const struct backlight_ops nv40_bl_ops = {
};
static int
-nv40_backlight_init(struct drm_connector *connector)
+nv40_backlight_init(struct nouveau_encoder *encoder,
+ struct backlight_properties *props,
+ const struct backlight_ops **ops)
{
- struct nouveau_drm *drm = nouveau_drm(connector->dev);
- struct nouveau_backlight *bl;
+ struct nouveau_drm *drm = nouveau_drm(encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
- struct backlight_properties props;
- char backlight_name[BL_NAME_SIZE];
- int ret = 0;
if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
- return 0;
-
- bl = kzalloc(sizeof(*bl), GFP_KERNEL);
- if (!bl)
- return -ENOMEM;
-
- memset(, 0, sizeof(struct backlight_properties));
- props.type = BACKLIGHT_RAW;
- props.max_brightness = 31;
- if (!nouveau_get_backlight_name(backlight_name, bl)) {
- NV_ERROR(drm, "Failed to retrieve a unique name for the
backlight interface\n");
- goto fail_alloc;
- }
-
- bl->dev = backlight_device_register(backlight_name, connector->kdev,
- drm, _bl_ops, );
- if (IS_ERR(bl->dev)) {
- if (bl->id >= 0)
- ida_simple_remove(_ida, bl->id);
-
- ret = PTR_ERR(bl->dev);
- goto fail_alloc;
- }
-
- nouveau_connector(connector)->backlight = bl;
- bl->dev->props.brightness = nv40_get_intensity(bl->dev);
- backlight_update_status(bl->dev);
+ return -ENODEV;
+ props->type = BACKLIGHT_RAW;
+ props->max_brightness = 31;
+ *ops = _bl_ops;
return 0;
-fail_alloc:
- kfree(bl);
- return ret;
}
static int
@@ -221,92 +195,102 @@ static const struct backlight_ops nva3_bl_ops = {
};
static int
-nv50_backlight_init(struct drm_connector *connector)
+nv50_backlight_init(struct nouveau_encoder *nv_encoder,
+ struct backlight_properties *props,
+ const struct backlight_ops **ops)
{
- struct nouveau_drm *drm = nouveau_drm(connector->dev);
+ struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
- struct nouveau_encoder *nv_encoder;
- struct nouveau_backlight *bl;
- struct backlight_properties props;
- const struct backlight_ops *ops;
- char backlight_name[BL_NAME_SIZE];
- int ret = 0;
-
- nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
- if (!nv_encoder) {
- nv_encoder = find_encoder(connector,
[PATCH v4 4/6] drm/nouveau: s/nouveau_backlight_exit/nouveau_backlight_fini/
More consistent with the rest of the codebase, no functional changes
here.
Signed-off-by: Lyude Paul
Reviewed-by: Karol Herbst
---
drivers/gpu/drm/nouveau/nouveau_backlight.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_connector.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 5d0694680f59..f4400a6408b4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -309,7 +309,7 @@ nouveau_backlight_init(struct drm_connector *connector)
}
void
-nouveau_backlight_exit(struct drm_connector *connector)
+nouveau_backlight_fini(struct drm_connector *connector)
{
struct nouveau_connector *nv_conn = nouveau_connector(connector);
struct nouveau_backlight *bl = nv_conn->backlight;
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 35c538f95167..1c0630bd13a3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -895,7 +895,7 @@ nouveau_connector_late_register(struct drm_connector
*connector)
static void
nouveau_connector_early_unregister(struct drm_connector *connector)
{
- nouveau_backlight_exit(connector);
+ nouveau_backlight_fini(connector);
}
static int
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index aefc3cbf8098..4de597ce60d0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -189,7 +189,7 @@ struct drm_display_mode *nouveau_conn_native_mode(struct
drm_connector *);
#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
extern int nouveau_backlight_init(struct drm_connector *);
-extern void nouveau_backlight_exit(struct drm_connector *);
+extern void nouveau_backlight_fini(struct drm_connector *);
extern void nouveau_backlight_ctor(void);
extern void nouveau_backlight_dtor(void);
#else
@@ -200,7 +200,7 @@ nouveau_backlight_init(struct drm_connector *connector)
}
static inline void
-nouveau_backlight_exit(struct drm_connector *connector) {
+nouveau_backlight_fini(struct drm_connector *connector) {
}
static inline void
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [RFC v2] Drop all 00-INDEX files from Documentation/
On Thu, Sep 6, 2018 at 6:01 PM, Steven Rostedt wrote: > On Thu, 6 Sep 2018 09:58:04 -0600 > Jonathan Corbet wrote: > >> Thanks, >> >> jon (who is increasingly inclined to apply this patch) > > As Colin Kaepernick now says... "Just do it!" > > ;-) +1 But I'm biased, I'm part of the party that is responsible for the new shiny documentation system ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 01/14] drm: add new plane property FB_DAMAGE_CLIPS to send damage during plane update
> >
> > +FB_DAMAGE_CLIPS
> > +~~~
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
> > + :doc: overview
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
> > + :export:
>
> You forgot to include your nice kerneldoc from the header. Please run
>
> $ make htmldocs
>
> and make sure the generated stuff looks all nice and is complete.
Thanks for the review Daniel, oops I added those docs later in
header and forgot to update here.
>
> > +
> > Color Management Properties
> > ---
> >
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index a6771cef85e2..ca5be0decb3f 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -35,7 +35,7 @@ drm_kms_helper-y := drm_crtc_helper.o
> drm_dp_helper.o drm_probe_helper.o \
> > drm_plane_helper.o drm_dp_mst_topology.o
> drm_atomic_helper.o \
> > drm_kms_helper_common.o drm_dp_dual_mode_helper.o
> \
> > drm_simple_kms_helper.o drm_modeset_helper.o \
> > - drm_scdc_helper.o drm_gem_framebuffer_helper.o
> > + drm_scdc_helper.o drm_gem_framebuffer_helper.o
> drm_damage_helper.o
> >
> > drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
> > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) +=
> drm_fb_helper.o
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> b/drivers/gpu/drm/drm_atomic.c
> > index 3eb061e11e2e..652e78ca1f81 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -857,6 +857,8 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> > {
> > struct drm_device *dev = plane->dev;
> > struct drm_mode_config *config = >mode_config;
> > + bool replaced = false;
> > + int ret;
> >
> > if (property == config->prop_fb_id) {
> > struct drm_framebuffer *fb =
> drm_framebuffer_lookup(dev, NULL, val);
> > @@ -908,6 +910,14 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> > state->color_encoding = val;
> > } else if (property == plane->color_range_property) {
> > state->color_range = val;
> > + } else if (property == config->prop_fb_damage_clips) {
> > + ret = drm_atomic_replace_property_blob_from_id(dev,
> > + >fb_damage_clips,
> > + val,
> > + -1,
> > + sizeof(struct drm_rect),
> > + );
> > + return ret;
> > } else if (plane->funcs->atomic_set_property) {
> > return plane->funcs->atomic_set_property(plane, state,
> > property, val);
> > @@ -976,6 +986,9 @@ drm_atomic_plane_get_property(struct drm_plane
> *plane,
> > *val = state->color_encoding;
> > } else if (property == plane->color_range_property) {
> > *val = state->color_range;
> > + } else if (property == config->prop_fb_damage_clips) {
> > + *val = (state->fb_damage_clips) ?
> > + state->fb_damage_clips->base.id : 0;
> > } else if (plane->funcs->atomic_get_property) {
> > return plane->funcs->atomic_get_property(plane, state,
> property, val);
> > } else {
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> > index 80be74df7ba6..be83e2763c18 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3576,6 +3576,7 @@ void drm_atomic_helper_plane_reset(struct
> drm_plane *plane)
> > /* Reset the alpha value to fully opaque if it matters */
> > if (plane->alpha_property)
> > plane->state->alpha = plane->alpha_property-
> >values[1];
> > + plane->state->fb_damage_clips = NULL;
>
> No need to set to 0, we require kzalloc.
>
> > }
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
> > @@ -3598,6 +3599,7 @@ void
> __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
> >
> > state->fence = NULL;
> > state->commit = NULL;
> > + state->fb_damage_clips = NULL;
> > }
> > EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
> >
> > @@ -3642,6 +3644,8 @@ void
> __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
> >
> > if (state->commit)
> > drm_crtc_commit_put(state->commit);
> > +
> > + drm_property_blob_put(state->fb_damage_clips);
> > }
> > EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
> >
> > diff --git a/drivers/gpu/drm/drm_damage_helper.c
> b/drivers/gpu/drm/drm_damage_helper.c
> > new file mode 100644
> > index ..3e7de5afe7f6
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_damage_helper.c
> > @@ -0,0 +1,92 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> >
> +/*
> *
> > + *
> > +
[Bug 106225] Kernel panic after modesetting (not on every boot) on ryzen 5 2400g
https://bugs.freedesktop.org/show_bug.cgi?id=106225 --- Comment #37 from Francisco Pina Martins --- Confirming that the issue seems to be solved with mainline linux-4.19-rc[1,2]. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 107784] [AMD tahiti XT] displayport broken
Got something, sort of obvious for a human, impossible for bisect to know, which explains why this bisection was a failure: testing a commit in the middle of a series of commits which are to be taken as a whole to be consistent for normal operations of the kernel, is wrong. That, bisect cannot know. In my case, the TSC commits are "good"... and testing time has nothing to do with this: testing a commit in the middle of this series will have a side effect (DP link/aux timings...) on what I am observing to decide if a commit is "bad" or "good". In my case it will break this observable (my DP monitor is _really_ not working) and I'll tag the commit as "bad", which is inconsistent. The bottom of this, which is _obvious_ for an experienced git user on large projects, at the time of big merges on the main branch of a project like linux, if something breaks, bisect is probably more your enemy than your friend: it is ok to ask "Can you bisect?" on a sub-sub-system branch which has been free from big merges from any related other subsystems for a good while, but almost an insult on such massive update. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107784] [AMD tahiti XT] displayport broken
https://bugs.freedesktop.org/show_bug.cgi?id=107784 --- Comment #18 from Sylvain BERTRAND --- Got something, sort of obvious for a human, impossible for bisect to know, which explains why this bisection was a failure: testing a commit in the middle of a series of commits which are to be taken as a whole to be consistent for normal operations of the kernel, is wrong. That, bisect cannot know. In my case, the TSC commits are "good"... and testing time has nothing to do with this: testing a commit in the middle of this series will have a side effect (DP link/aux timings...) on what I am observing to decide if a commit is "bad" or "good". In my case it will break this observable (my DP monitor is _really_ not working) and I'll tag the commit as "bad", which is inconsistent. The bottom of this, which is _obvious_ for an experienced git user on large projects, at the time of big merges on the main branch of a project like linux, if something breaks, bisect is probably more your enemy than your friend: it is ok to ask "Can you bisect?" on a sub-sub-system branch which has been free from big merges from any related other subsystems for a good while, but almost an insult on such massive update. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/2] drm: Add detection of changing of edid on between suspend and resume
The hotplug detection routine of drm_helper_hpd_irq_event() can detect
changing of status of connector, but it can not detect changing of edid.
Following scenario requires detection of changing of edid.
1) plug display device to a connector
2) system suspend
3) unplug 1)'s display device and plug the other display device to a
connector
4) system resume
It adds edid check routine when a connector status still remains as
"connector_status_connected".
v2: Add NULL check before comparing of EDIDs.
v3: Make it as part of existing drm_helper_hpd_irq_event() (Stan, Mika)
Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
Signed-off-by: Gwan-gyeong Mun
---
drivers/gpu/drm/drm_probe_helper.c | 29 -
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index a1bb157bfdfa..2705a5a0e4d6 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -742,7 +742,16 @@ EXPORT_SYMBOL(drm_kms_helper_poll_fini);
* panels.
*
* This helper function is useful for drivers which can't or don't track
hotplug
- * interrupts for each connector.
+ * interrupts for each connector. And it also supports a detection of changing
+ * of edid on between suspend and resume when a connector status still remains
+ * as "connector_status_connected".
+ *
+ * Following scenario requires detection of changing of edid.
+ * 1) plug display device to a connector
+ * 2) system suspend
+ * 3) unplug 1)'s display device and plug the other display device to a
+ connector
+ * 4) system resume
*
* Drivers which support hotplug interrupts for each connector individually and
* which have a more fine-grained detect logic should bypass this code and
@@ -760,6 +769,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
enum drm_connector_status old_status;
+ struct edid *old_edid;
bool changed = false;
if (!dev->mode_config.poll_enabled)
@@ -773,6 +783,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
continue;
old_status = connector->status;
+ old_edid = connector->detect_edid;
connector->status = drm_helper_probe_detect(connector, NULL,
false);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
%s\n",
@@ -782,6 +793,22 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
drm_get_connector_status_name(connector->status));
if (old_status != connector->status)
changed = true;
+
+ /* Check changing of edid when a connector status still remains
+* as "connector_status_connected".
+*/
+ if (old_status == connector->status &&
+ old_status == connector_status_connected) {
+ if (!old_edid || !connector->detect_edid)
+ continue;
+
+ if (memcmp(old_edid, connector->detect_edid,
sizeof(*old_edid))) {
+ changed = true;
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] edid
updated\n",
+ connector->base.id,
+ connector->name);
+ }
+ }
}
drm_connector_list_iter_end(_iter);
mutex_unlock(>mode_config.mutex);
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/2] drm: move a detected edid member to drm_connector from intel_connector
In order to use a detected edid on drm helper functions, it moves
a detected edid member to drm_connector structure from intel_connector
structure.
Signed-off-by: Gwan-gyeong Mun
---
drivers/gpu/drm/i915/intel_dp.c | 18 +-
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_hdmi.c | 10 +-
include/drm/drm_connector.h | 7 +++
4 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 436c22de33b6..c117f552f7d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4157,7 +4157,7 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
*intel_dp)
struct intel_connector *intel_connector = intel_dp->attached_connector;
struct drm_connector *connector = _connector->base;
- if (intel_connector->detect_edid == NULL ||
+ if (connector->detect_edid == NULL ||
connector->edid_corrupt ||
intel_dp->aux.i2c_defer_count > 6) {
/* Check EDID read for NACKs, DEFERs and corruption
@@ -4174,12 +4174,12 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
*intel_dp)
intel_dp->aux.i2c_defer_count);
intel_dp->compliance.test_data.edid =
INTEL_DP_RESOLUTION_FAILSAFE;
} else {
- struct edid *block = intel_connector->detect_edid;
+ struct edid *block = connector->detect_edid;
/* We have to write the checksum
* of the last block read
*/
- block += intel_connector->detect_edid->extensions;
+ block += connector->detect_edid->extensions;
if (drm_dp_dpcd_writeb(_dp->aux, DP_TEST_EDID_CHECKSUM,
block->checksum) <= 0)
@@ -4993,7 +4993,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
intel_dp_unset_edid(intel_dp);
edid = intel_dp_get_edid(intel_dp);
- intel_connector->detect_edid = edid;
+ intel_connector->base.detect_edid = edid;
intel_dp->has_audio = drm_detect_monitor_audio(edid);
drm_dp_cec_set_edid(_dp->aux, edid);
@@ -5005,8 +5005,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
struct intel_connector *intel_connector = intel_dp->attached_connector;
drm_dp_cec_unset_edid(_dp->aux);
- kfree(intel_connector->detect_edid);
- intel_connector->detect_edid = NULL;
+ kfree(intel_connector->base.detect_edid);
+ intel_connector->base.detect_edid = NULL;
intel_dp->has_audio = false;
}
@@ -5099,7 +5099,7 @@ intel_dp_long_pulse(struct intel_connector *connector,
intel_dp->aux.i2c_defer_count = 0;
intel_dp_set_edid(intel_dp);
- if (intel_dp_is_edp(intel_dp) || connector->detect_edid)
+ if (intel_dp_is_edp(intel_dp) || connector->base.detect_edid)
status = connector_status_connected;
intel_dp->detect_done = true;
@@ -5183,7 +5183,7 @@ static int intel_dp_get_modes(struct drm_connector
*connector)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct edid *edid;
- edid = intel_connector->detect_edid;
+ edid = connector->detect_edid;
if (edid) {
int ret = intel_connector_update_modes(connector, edid);
if (ret)
@@ -5245,7 +5245,7 @@ intel_dp_connector_destroy(struct drm_connector
*connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
- kfree(intel_connector->detect_edid);
+ kfree(connector->detect_edid);
if (!IS_ERR_OR_NULL(intel_connector->edid))
kfree(intel_connector->edid);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5731215210a..19edd7ec4138 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -400,7 +400,6 @@ struct intel_connector {
/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
struct edid *edid;
- struct edid *detect_edid;
/* since POLL and HPD connectors may use the same HPD line keep the
native
state of connector->polled in case hotplug storm detection changes
it */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index a2dab0b6bde6..0bedfc0ade49 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1815,8 +1815,8 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
- kfree(to_intel_connector(connector)->detect_edid);
- to_intel_connector(connector)->detect_edid = NULL;
+ kfree(connector->detect_edid);
+ connector->detect_edid = NULL;
}
static void
@@ -1892,7 +1892,7 @@
Re: [PATCH 1/6] drm/bridge: use bus flags in bridge timings
On 06.09.2018 04:07, Linus Walleij wrote: > On Wed, Sep 5, 2018 at 8:32 PM Stefan Agner wrote: >> On 05.09.2018 00:44, Laurent Pinchart wrote: > >> Good point! I actually really don't like that we use the same flags here >> but from a different perspective. Especially since the flags defines >> document things differently: >> >> /* drive data on pos. edge */ >> #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) >> /* drive data on neg. edge */ >> #define DRM_BUS_FLAG_PIXDATA_NEGEDGE(1<<3) > > Maybe a stupid comment from my side, but can't we just change the > documentation to match the usecases? > > /* Trigger pixel data latch on positive edge */ > #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) > >> Using the opposite perspective would also need translation in crtc >> drivers... So far no driver uses sampling_edge. >> >> I would prefer if we always use the meaning as documented by the flags. >> >> I guess we would need to convert DRM_BUS_FLAG_PIXDATA_POSEDGE -> >> DRM_BUS_FLAG_PIXDATA_NEGEDGE. >> >> Linus Walleij, you added sampling edge, any thoughts? > > I just thought it was generally useful to have triggering edge encoded > into the bridge as it makes it clear that this edge is something > that is a delayed version of the driving edge which is subject to > clock skew caused by the speed of electrons in silicon and > copper and slew rate caused by parasitic capacitance. Ok, I read a bit up on the history of bridge timing, especially: https://www.spinics.net/lists/dri-devel/msg155618.html IMHO, this got overengineered. For displays we do not need all that setup/sample delay timing information, and much longer cables are in use. So why is all that needed for bridges? For Linus case, the THS8134(A/B) data sheet I found (revised March 2010) clearly states: Clock input. A rising edge on CLK latches RPr0-7, GY0-7, BPb0-7. So we need to drive on negative edge, hence DRM_BUS_FLAG_PIXDATA_NEGEDGE should be used, which makes the pl111 driver setting TIM2_IPC: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0121d/index.html > DRM_BUS_FLAG_PIXDATA_POSEDGE is the right value for my use cases, but it > doesn't match how the ADV7123 operates. Using DRM_BUS_FLAG_PIXDATA_NEGEDGE > would match the hardware, but would break display for some modes, depending > on > the display clock frequency as the internal 8.5ns output delay applied to a > falling clock edge would fall right into the 1.7ns setup + hold time window > of > the ADV7123 around the rising edge. I can't test this right now as I don't > have local access to boards using the ADV7123, but from a quick calculation > that ignores the PCB transmission delay modes with frequencies between 57MHz > and 71MHz could break if the data was output on the falling edge of the clock. If clocks vs. data signal are really that much off on R-Car DU, then parallel displays must have the very same issue... Are you sure that only the clock signal has an output delay? And that this output delay is a fixed value, clock independent? Typically, delays apply to all signals equally, and often are clock frequency dependent... Without really looking at the signals, I would not jump to conclusions here! I am pretty sure that driving on negative edge works just as well. -- Stefan > > Yours, > Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] intel: annotate the intel genx helpers as private
On Thu, Sep 06, 2018 at 06:59:33PM +0100, Chris Wilson wrote:
> Quoting Lucas De Marchi (2018-09-06 18:51:37)
> > On Thu, Sep 06, 2018 at 06:38:52PM +0100, Chris Wilson wrote:
> > > Quoting Emil Velikov (2018-09-06 16:14:07)
> > > > From: Emil Velikov
> > > >
> > > > They're used internally and never meant to be part of the API.
> > > > Add the drm_private notation, which should resolve that.
> > > >
> > > > Cc: Eric Engestrom
> > > > Cc: Lucas De Marchi
> > > > Cc: Chris Wilson
> > > > Cc: Rodrigo Vivi
> > > > Fixes: 4e81d4f9c9b ("intel: add generic functions to check PCI ID")
> > > > Signed-off-by: Emil Velikov
> > > > ---
> > > > intel/intel_chipset.c | 4 ++--
> > > > intel/intel_chipset.h | 4 ++--
> > > > 2 files changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
> > > > index d5c33cc5..5aa4a2f2 100644
> > > > --- a/intel/intel_chipset.c
> > > > +++ b/intel/intel_chipset.c
> > > > @@ -44,7 +44,7 @@ static const struct pci_device {
> > > > INTEL_SKL_IDS(9),
> > > > };
> > > >
> > > > -bool intel_is_genx(unsigned int devid, int gen)
> > > > +drm_private bool intel_is_genx(unsigned int devid, int gen)
> > > > {
> > > > const struct pci_device *p,
> > > > *pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
> > > > @@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
> > > > return false;
> > > > }
> > > >
> > > > -bool intel_get_genx(unsigned int devid, int *gen)
> > > > +drm_private bool intel_get_genx(unsigned int devid, int *gen)
> > >
> > > We should only need to put the attribute in the header, right?
> >
> > IMO it actually makes more sense to be in the .c. Reason is that if we
> > are going to change to be hidden by default and annotate the public
> > ones, then we don't need to play with macros to hide them from other
> > projects including the header.
> >
> > A declaration for a private symbol should not be in an exported header
> > so we know that all the functions in that header should actually be
> > public.
>
> And we definitely should not be and are not exporting intel_chipset.h.
>
> I'd would rather have visibility declared in the header because that's
> where I expect interface documentation.
but then if the header is exported, you need to export the definition of
the macro as well.. and undef it when it's not for internal use. I've
seen nasty things on projects that went this route, because then
you not only depend on the compiler version you are compiled with, but
you also need to keep the flexibility for other projects that are
including you. Example:
#ifdef EAPI
# undef EAPI
#endif
#ifdef _WIN32
# ifdef EFL_BUILD
# ifdef DLL_EXPORT
# define EAPI __declspec(dllexport)
# else
# define EAPI
# endif
# else
# define EAPI __declspec(dllimport)
# endif
# define EAPI_WEAK
#else
# ifdef __GNUC__
# if __GNUC__ >= 4
# define EAPI __attribute__ ((visibility("default")))
# define EAPI_WEAK __attribute__ ((weak))
# else
# define EAPI
# define EAPI_WEAK
# endif
# else
# define EAPI
# define EAPI_WEAK
# endif
#endif
Lucas De Marchi
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107693] [wine] Wolfenstein: The Old Blood - can't find GL_EXT_framebuffer_object
https://bugs.freedesktop.org/show_bug.cgi?id=107693 --- Comment #7 from gloriouseggr...@gmail.com --- this patch was for wine btw, not mesa, but maybe it can be used to find what wine was providing that allowed the game to start -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107693] [wine] Wolfenstein: The Old Blood - can't find GL_EXT_framebuffer_object
https://bugs.freedesktop.org/show_bug.cgi?id=107693 --- Comment #6 from gloriouseggr...@gmail.com --- Unsure if this helps or not, but before mesa's profiles were updated I was able to get the game running using this patch: https://gitlab.com/GloriousEggroll/wolfenstein-tno-tob-linux/blob/master/idtech5.patch and setting MESA_GL_VERSION_OVERRIDE=4.5COMPAT this was working with wine 2.17 at the time -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201035] I2C bus probing crash on OpenChrome DRM
https://bugzilla.kernel.org/show_bug.cgi?id=201035 --- Comment #1 from Kevin Brace (kevinbr...@gmx.com) --- Created attachment 278351 --> https://bugzilla.kernel.org/attachment.cgi?id=278351=edit kern.log for VIA Embedded EPIA-M830 mainboard -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201035] I2C bus probing crash on OpenChrome DRM
https://bugzilla.kernel.org/show_bug.cgi?id=201035 Kevin Brace (kevinbr...@gmx.com) changed: What|Removed |Added URL||https://cgit.freedesktop.or ||g/openchrome/drm-openchrome ||/log/?h=drm-next-4.20 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201035] New: I2C bus probing crash on OpenChrome DRM
https://bugzilla.kernel.org/show_bug.cgi?id=201035 Bug ID: 201035 Summary: I2C bus probing crash on OpenChrome DRM Product: Drivers Version: 2.5 Kernel Version: 4.19 rc1 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: blocking Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: kevinbr...@gmx.com Regression: No OpenChrome DRM was working fine (i.e., able to normally boot Xubuntu 16.04) until DRM maintainer's version of Linux 4.18 rc4. When I upgraded to Linux 4.18 rc7, it stopped working. I also upgraded to Linux 4.19 rc1, but it crashes for a similar reason. It appears that there is a NULL pointer reference when probing for I2C bus. The bug was observed on HP 2133 mini-note and VIA Embedded EPIA-M830 mainboard. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105018] Kernel panic when waking up after screen goes blank.
https://bugs.freedesktop.org/show_bug.cgi?id=105018 ecomer changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #39 from ecomer --- I have this same problem wirh 4.18.5-1-MANJARO x86_64 $ inxi CPU: Dual Core AMD A6-9500 RADEON R5 8 COMPUTE CORES 2C+6G (-MCP-) speed/min/max: 1622/1400/3500 MHz Kernel: 4.18.5-1-MANJARO x86_64 Up: 14m Mem: 1365.6/15029.9 MiB (9.1%) Storage: 1.93 TiB (23.8% used) Procs: 189 Shell: bash 4.4.23 inxi: 3.0.21 Running an XFCE desktop. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/nouveau/debugfs: fix pm_runtime.cocci warnings
From: kbuild test robot
pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at line 164
Generated by: scripts/coccinelle/api/pm_runtime.cocci
Fixes: eaeb9010bb4b ("drm/nouveau/debugfs: Wake up GPU before doing any
reclocking")
CC: Karol Herbst
Signed-off-by: kbuild test robot
Signed-off-by: Julia Lawall
---
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: b36fdc6853a38a6f8749897a33435635019e0647
commit: eaeb9010bb4bcdc20e58254fa42f3fe730a7f908 drm/nouveau/debugfs: Wake up
GPU before doing any reclocking
:: branch date: 21 hours ago
:: commit date: 7 weeks ago
nouveau_debugfs.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -161,7 +161,7 @@ nouveau_debugfs_pstate_set(struct file *
}
ret = pm_runtime_get_sync(drm->dev);
- if (IS_ERR_VALUE(ret) && ret != -EACCES)
+ if (ret < 0 && ret != -EACCES)
return ret;
ret = nvif_mthd(ctrl, NVIF_CONTROL_PSTATE_USER, , sizeof(args));
pm_runtime_put_autosuspend(drm->dev);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] intel: annotate the intel genx helpers as private
Quoting Lucas De Marchi (2018-09-06 18:51:37)
> On Thu, Sep 06, 2018 at 06:38:52PM +0100, Chris Wilson wrote:
> > Quoting Emil Velikov (2018-09-06 16:14:07)
> > > From: Emil Velikov
> > >
> > > They're used internally and never meant to be part of the API.
> > > Add the drm_private notation, which should resolve that.
> > >
> > > Cc: Eric Engestrom
> > > Cc: Lucas De Marchi
> > > Cc: Chris Wilson
> > > Cc: Rodrigo Vivi
> > > Fixes: 4e81d4f9c9b ("intel: add generic functions to check PCI ID")
> > > Signed-off-by: Emil Velikov
> > > ---
> > > intel/intel_chipset.c | 4 ++--
> > > intel/intel_chipset.h | 4 ++--
> > > 2 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
> > > index d5c33cc5..5aa4a2f2 100644
> > > --- a/intel/intel_chipset.c
> > > +++ b/intel/intel_chipset.c
> > > @@ -44,7 +44,7 @@ static const struct pci_device {
> > > INTEL_SKL_IDS(9),
> > > };
> > >
> > > -bool intel_is_genx(unsigned int devid, int gen)
> > > +drm_private bool intel_is_genx(unsigned int devid, int gen)
> > > {
> > > const struct pci_device *p,
> > > *pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
> > > @@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
> > > return false;
> > > }
> > >
> > > -bool intel_get_genx(unsigned int devid, int *gen)
> > > +drm_private bool intel_get_genx(unsigned int devid, int *gen)
> >
> > We should only need to put the attribute in the header, right?
>
> IMO it actually makes more sense to be in the .c. Reason is that if we
> are going to change to be hidden by default and annotate the public
> ones, then we don't need to play with macros to hide them from other
> projects including the header.
>
> A declaration for a private symbol should not be in an exported header
> so we know that all the functions in that header should actually be
> public.
And we definitely should not be and are not exporting intel_chipset.h.
I'd would rather have visibility declared in the header because that's
where I expect interface documentation.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 --- Comment #9 from Christian König --- (In reply to Michel Dänzer from comment #7) > Hmm, 'fork' sounded suspicious, and indeed AFAICT this test uses the same > amdgpu_context_handle (and by extension the same DRM file descriptor) in > multiple processes spawned by fork(). I'm afraid Christian is going to > explain why that's not supported. :) Actually the kernel driver can work with that. It isn't supported by Mesa. Christian. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] intel: annotate the intel genx helpers as private
On Thu, Sep 06, 2018 at 06:38:52PM +0100, Chris Wilson wrote:
> Quoting Emil Velikov (2018-09-06 16:14:07)
> > From: Emil Velikov
> >
> > They're used internally and never meant to be part of the API.
> > Add the drm_private notation, which should resolve that.
> >
> > Cc: Eric Engestrom
> > Cc: Lucas De Marchi
> > Cc: Chris Wilson
> > Cc: Rodrigo Vivi
> > Fixes: 4e81d4f9c9b ("intel: add generic functions to check PCI ID")
> > Signed-off-by: Emil Velikov
> > ---
> > intel/intel_chipset.c | 4 ++--
> > intel/intel_chipset.h | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
> > index d5c33cc5..5aa4a2f2 100644
> > --- a/intel/intel_chipset.c
> > +++ b/intel/intel_chipset.c
> > @@ -44,7 +44,7 @@ static const struct pci_device {
> > INTEL_SKL_IDS(9),
> > };
> >
> > -bool intel_is_genx(unsigned int devid, int gen)
> > +drm_private bool intel_is_genx(unsigned int devid, int gen)
> > {
> > const struct pci_device *p,
> > *pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
> > @@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
> > return false;
> > }
> >
> > -bool intel_get_genx(unsigned int devid, int *gen)
> > +drm_private bool intel_get_genx(unsigned int devid, int *gen)
>
> We should only need to put the attribute in the header, right?
IMO it actually makes more sense to be in the .c. Reason is that if we
are going to change to be hidden by default and annotate the public
ones, then we don't need to play with macros to hide them from other
projects including the header.
A declaration for a private symbol should not be in an exported header
so we know that all the functions in that header should actually be
public.
Lucas De Marchi
> -Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] intel: annotate the intel genx helpers as private
On Thu, Sep 06, 2018 at 04:14:07PM +0100, Emil Velikov wrote:
> From: Emil Velikov
>
> They're used internally and never meant to be part of the API.
> Add the drm_private notation, which should resolve that.
>
> Cc: Eric Engestrom
> Cc: Lucas De Marchi
> Cc: Chris Wilson
> Cc: Rodrigo Vivi
> Fixes: 4e81d4f9c9b ("intel: add generic functions to check PCI ID")
> Signed-off-by: Emil Velikov
> ---
> intel/intel_chipset.c | 4 ++--
> intel/intel_chipset.h | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
> index d5c33cc5..5aa4a2f2 100644
> --- a/intel/intel_chipset.c
> +++ b/intel/intel_chipset.c
> @@ -44,7 +44,7 @@ static const struct pci_device {
> INTEL_SKL_IDS(9),
> };
>
> -bool intel_is_genx(unsigned int devid, int gen)
> +drm_private bool intel_is_genx(unsigned int devid, int gen)
> {
> const struct pci_device *p,
> *pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
> @@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
> return false;
> }
>
> -bool intel_get_genx(unsigned int devid, int *gen)
> +drm_private bool intel_get_genx(unsigned int devid, int *gen)
> {
> const struct pci_device *p,
> *pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
> diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
> index 9b1e64f1..63ddde7a 100644
> --- a/intel/intel_chipset.h
> +++ b/intel/intel_chipset.h
> @@ -330,8 +330,8 @@
> /* New platforms use kernel pci ids */
> #include
>
> -bool intel_is_genx(unsigned int devid, int gen);
> -bool intel_get_genx(unsigned int devid, int *gen);
> +drm_private bool intel_is_genx(unsigned int devid, int gen);
> +drm_private bool intel_get_genx(unsigned int devid, int *gen);
>
> #define IS_GEN9(devid) intel_is_genx(devid, 9)
> #define IS_GEN10(devid) intel_is_genx(devid, 10)
> --
As a short-term fix,
Acked-by: Lucas De Marchi
I think we should really migrate to making them hidden by default and
only export the ones we want. When drm_public was removed, it was
basically a nop since the visibility was still set as default. I will
take a look on this.
Lucas De Marchi
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] intel: annotate the intel genx helpers as private
Quoting Emil Velikov (2018-09-06 16:14:07)
> From: Emil Velikov
>
> They're used internally and never meant to be part of the API.
> Add the drm_private notation, which should resolve that.
>
> Cc: Eric Engestrom
> Cc: Lucas De Marchi
> Cc: Chris Wilson
> Cc: Rodrigo Vivi
> Fixes: 4e81d4f9c9b ("intel: add generic functions to check PCI ID")
> Signed-off-by: Emil Velikov
> ---
> intel/intel_chipset.c | 4 ++--
> intel/intel_chipset.h | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
> index d5c33cc5..5aa4a2f2 100644
> --- a/intel/intel_chipset.c
> +++ b/intel/intel_chipset.c
> @@ -44,7 +44,7 @@ static const struct pci_device {
> INTEL_SKL_IDS(9),
> };
>
> -bool intel_is_genx(unsigned int devid, int gen)
> +drm_private bool intel_is_genx(unsigned int devid, int gen)
> {
> const struct pci_device *p,
> *pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
> @@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
> return false;
> }
>
> -bool intel_get_genx(unsigned int devid, int *gen)
> +drm_private bool intel_get_genx(unsigned int devid, int *gen)
We should only need to put the attribute in the header, right?
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] drm/bridge: use bus flags in bridge timings
On 06.09.2018 09:54, Laurent Pinchart wrote: > Hi Stefan, > > On Thursday, 6 September 2018 19:48:51 EEST Stefan Agner wrote: >> On 06.09.2018 05:25, Laurent Pinchart wrote: >> > On Thursday, 6 September 2018 14:07:41 EEST Linus Walleij wrote: >> >> On Wed, Sep 5, 2018 at 8:32 PM Stefan Agner wrote: >> >> > On 05.09.2018 00:44, Laurent Pinchart wrote: >> >> > >> >> > Good point! I actually really don't like that we use the same flags >> >> > here >> >> > but from a different perspective. Especially since the flags defines >> >> > document things differently: >> >> > >> >> > /* drive data on pos. edge */ >> >> > #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) >> >> > /* drive data on neg. edge */ >> >> > #define DRM_BUS_FLAG_PIXDATA_NEGEDGE(1<<3) >> >> >> >> Maybe a stupid comment from my side, but can't we just change the >> >> documentation to match the usecases? >> >> >> >> /* Trigger pixel data latch on positive edge */ >> >> #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) >> > >> > The flags are used for the drm_connector bus_flags field, and really mean >> > driving on the positive/negative edges. We this can't change their meaning >> > like this. >> > >> >> > Using the opposite perspective would also need translation in crtc >> >> > drivers... So far no driver uses sampling_edge. >> >> > >> >> > I would prefer if we always use the meaning as documented by the flags. >> >> > >> >> > I guess we would need to convert DRM_BUS_FLAG_PIXDATA_POSEDGE -> >> >> > DRM_BUS_FLAG_PIXDATA_NEGEDGE. >> >> > >> >> > Linus Walleij, you added sampling edge, any thoughts? >> >> >> >> I just thought it was generally useful to have triggering edge encoded >> >> into the bridge as it makes it clear that this edge is something >> >> that is a delayed version of the driving edge which is subject to >> >> clock skew caused by the speed of electrons in silicon and >> >> copper and slew rate caused by parasitic capacitance. >> > >> > I agree that we need both the driving and sampling edge. In many case they >> > will be opposite, and providing some kind of appropriate defaults in APIs >> > is fine by me, but we need a way to specify both when needed. >> >> We do have pixel clock flags for displays, but also they are actually >> controller oriented: >> >> https://elixir.bootlin.com/linux/latest/source/include/video/display_timing. >> h#L15 >> >> I guess having different flags to denote driving and sampling edge >> independently would be ideal. But then, is there really a use case where >> it wouldn't be the exact opposite? > > Yes, for instance when the transmission delay for clock and data signals is > different, you may want to sample on the same edge used by the transmitter to > latch the output. Linus had that case, which prompted him to add the sampling > edge field. > Bu in that case the controller does not know what it actually should do with the sampling information alone either... Where is that code handling that today? sample_edge has been added, but I only see setup_time_ps being used in pl111_display.c. I'm not a hardware guy, but according to my rough calculation a pixel clock signal at 100MHz would need to be 1.5m off compared to the data signals to skew by a half period! Length matching is usually only necessary at really high speed signals. Is that really what is going on here? Why not just work around by specifying "the wrong" edge...? -- Stefan >> The other bus flags are actually fine as is. I suggest to just stick >> with the bus flags as we have them now, at least for now. >> >> Alternatively, we could provide "consumer/bridge" oriented flags which >> use the same bit and just are the opposite of the controller oriented >> flags, e.g.: >> >> /* drive data on pos. edge */ >> #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) >> /* drive data on neg. edge */ >> #define DRM_BUS_FLAG_PIXDATA_NEGEDGE (1<<3) >> /* sample data on neg. edge */ >> #define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE (1<<2) >> /* sample data on pos. edge */ >> #define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE (1<<3) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/10] phy: Add configuration interface
> > > +int phy_configure(struct phy *phy, enum phy_mode mode,
> > > + union phy_configure_opts *opts)
> > > +{
> > > + int ret;
> > > +
> > > + if (!phy)
> > > + return -EINVAL;
> > > +
> > > + if (!phy->ops->configure)
> > > + return 0;
> >
> > Shouldn't you report an error to the caller ? If a caller expects the PHY
> > to
> > be configurable, I would assume that silently ignoring the requested
> > configuration won't work great.
>
> I'm not sure. I also expect a device having to interact with multiple
> PHYs, some of them needing some configuration while some other do
> not. In that scenario, returning 0 seems to be the right thing to do.
You could return -EOPNOTSUPP. That is common in the network stack. The
caller then has the information to decide if it should keep going, or
return an error.
Andrew
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] drm/bridge: use bus flags in bridge timings
Hi Stefan, On Thursday, 6 September 2018 19:48:51 EEST Stefan Agner wrote: > On 06.09.2018 05:25, Laurent Pinchart wrote: > > On Thursday, 6 September 2018 14:07:41 EEST Linus Walleij wrote: > >> On Wed, Sep 5, 2018 at 8:32 PM Stefan Agner wrote: > >> > On 05.09.2018 00:44, Laurent Pinchart wrote: > >> > > >> > Good point! I actually really don't like that we use the same flags > >> > here > >> > but from a different perspective. Especially since the flags defines > >> > document things differently: > >> > > >> > /* drive data on pos. edge */ > >> > #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) > >> > /* drive data on neg. edge */ > >> > #define DRM_BUS_FLAG_PIXDATA_NEGEDGE(1<<3) > >> > >> Maybe a stupid comment from my side, but can't we just change the > >> documentation to match the usecases? > >> > >> /* Trigger pixel data latch on positive edge */ > >> #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) > > > > The flags are used for the drm_connector bus_flags field, and really mean > > driving on the positive/negative edges. We this can't change their meaning > > like this. > > > >> > Using the opposite perspective would also need translation in crtc > >> > drivers... So far no driver uses sampling_edge. > >> > > >> > I would prefer if we always use the meaning as documented by the flags. > >> > > >> > I guess we would need to convert DRM_BUS_FLAG_PIXDATA_POSEDGE -> > >> > DRM_BUS_FLAG_PIXDATA_NEGEDGE. > >> > > >> > Linus Walleij, you added sampling edge, any thoughts? > >> > >> I just thought it was generally useful to have triggering edge encoded > >> into the bridge as it makes it clear that this edge is something > >> that is a delayed version of the driving edge which is subject to > >> clock skew caused by the speed of electrons in silicon and > >> copper and slew rate caused by parasitic capacitance. > > > > I agree that we need both the driving and sampling edge. In many case they > > will be opposite, and providing some kind of appropriate defaults in APIs > > is fine by me, but we need a way to specify both when needed. > > We do have pixel clock flags for displays, but also they are actually > controller oriented: > > https://elixir.bootlin.com/linux/latest/source/include/video/display_timing. > h#L15 > > I guess having different flags to denote driving and sampling edge > independently would be ideal. But then, is there really a use case where > it wouldn't be the exact opposite? Yes, for instance when the transmission delay for clock and data signals is different, you may want to sample on the same edge used by the transmitter to latch the output. Linus had that case, which prompted him to add the sampling edge field. > The other bus flags are actually fine as is. I suggest to just stick > with the bus flags as we have them now, at least for now. > > Alternatively, we could provide "consumer/bridge" oriented flags which > use the same bit and just are the opposite of the controller oriented > flags, e.g.: > > /* drive data on pos. edge */ > #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) > /* drive data on neg. edge */ > #define DRM_BUS_FLAG_PIXDATA_NEGEDGE (1<<3) > /* sample data on neg. edge */ > #define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE (1<<2) > /* sample data on pos. edge */ > #define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE (1<<3) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/10] phy: Add configuration interface
Hi Maxime,
On Thursday, 6 September 2018 17:48:07 EEST Maxime Ripard wrote:
> On Wed, Sep 05, 2018 at 04:39:46PM +0300, Laurent Pinchart wrote:
> > On Wednesday, 5 September 2018 12:16:33 EEST Maxime Ripard wrote:
> >> The phy framework is only allowing to configure the power state of the
> >> PHY using the init and power_on hooks, and their power_off and exit
> >> counterparts.
> >>
> >> While it works for most, simple, PHYs supported so far, some more
> >> advanced PHYs need some configuration depending on runtime parameters.
> >> These PHYs have been supported by a number of means already, often by
> >> using ad-hoc drivers in their consumer drivers.
> >>
> >> That doesn't work too well however, when a consumer device needs to deal
> >
> > s/deal/deal with/
> >
> >> multiple PHYs, or when multiple consumers need to deal with the same PHY
> >> (a DSI driver and a CSI driver for example).
> >>
> >> So we'll add a new interface, through two funtions, phy_validate and
> >> phy_configure. The first one will allow to check that a current
> >> configuration, for a given mode, is applicable. It will also allow the
> >> PHY driver to tune the settings given as parameters as it sees fit.
> >>
> >> phy_configure will actually apply that configuration in the phy itself.
> >>
> >> Signed-off-by: Maxime Ripard
> >> ---
> >>
> >> drivers/phy/phy-core.c | 62 +-
> >> include/linux/phy/phy.h | 42 -
> >> 2 files changed, 104 insertions(+)
> >>
> >> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> >> index 35fd38c5a4a1..6eaf655e370f 100644
> >> --- a/drivers/phy/phy-core.c
> >> +++ b/drivers/phy/phy-core.c
> >> @@ -408,6 +408,68 @@ int phy_calibrate(struct phy *phy)
> >> EXPORT_SYMBOL_GPL(phy_calibrate);
> >>
> >> /**
> >> + * phy_configure() - Changes the phy parameters
> >> + * @phy: the phy returned by phy_get()
> >> + * @mode: phy_mode the configuration is applicable to.
> >> + * @opts: New configuration to apply
> >> + *
> >> + * Used to change the PHY parameters. phy_init() must have
> >> + * been called on the phy.
> >> + *
> >> + * Returns: 0 if successful, an negative error code otherwise
> >> + */
> >> +int phy_configure(struct phy *phy, enum phy_mode mode,
> >> +union phy_configure_opts *opts)
> >> +{
> >> + int ret;
> >> +
> >> + if (!phy)
> >> + return -EINVAL;
> >> +
> >> + if (!phy->ops->configure)
> >> + return 0;
> >
> > Shouldn't you report an error to the caller ? If a caller expects the PHY
> > to be configurable, I would assume that silently ignoring the requested
> > configuration won't work great.
>
> I'm not sure. I also expect a device having to interact with multiple
> PHYs, some of them needing some configuration while some other do
> not. In that scenario, returning 0 seems to be the right thing to do.
It could be up to the caller to decide whether to ignore the error or not when
the operation isn't implemented. I expect that a call requiring specific
configuration parameters for a given PHY might want to bail out if the
configuration can't be applied. On the other hand that should never happen
when the system is designed correctly, as vendors are not supposed to ship
kernels that would be broken by design (as in requiring a configure operation
but not providing it).
> >> + mutex_lock(>mutex);
> >> + ret = phy->ops->configure(phy, mode, opts);
> >> + mutex_unlock(>mutex);
> >> +
> >> + return ret;
> >> +}
[snip]
> >> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> >> index 9cba7fe16c23..3cc315dcfcd0 100644
> >> --- a/include/linux/phy/phy.h
> >> +++ b/include/linux/phy/phy.h
[snip]
> >> @@ -60,6 +66,38 @@ struct phy_ops {
> >>int (*power_on)(struct phy *phy);
> >>int (*power_off)(struct phy *phy);
> >>int (*set_mode)(struct phy *phy, enum phy_mode mode);
> >> +
> >> + /**
> >> + * @configure:
> >> + *
> >> + * Optional.
> >> + *
> >> + * Used to change the PHY parameters. phy_init() must have
> >> + * been called on the phy.
> >> + *
> >> + * Returns: 0 if successful, an negative error code otherwise
> >> + */
> >> + int (*configure)(struct phy *phy, enum phy_mode mode,
> >> + union phy_configure_opts *opts);
> >
> > Is this function allowed to modify opts ? If so, to what extent ? If not,
> > the pointer should be made const.
>
> That's a pretty good question. I guess it could modify it to the same
> extent than validate could. Would that make sense?
It would, or we could say that PHY users are required to call the validate
function first, and the the configure function will return an error if the
passed configuration isn't valid. That would avoid double-validation when the
PHY user uses .validate().
> >> + /**
> >> + * @validate:
> >> + *
> >> + * Optional.
> >> + *
> >> + * Used to check that the current set of parameters can be
> >> + *
Re: [PATCH 1/6] drm/bridge: use bus flags in bridge timings
On 06.09.2018 05:25, Laurent Pinchart wrote: > Hi Linus, > > On Thursday, 6 September 2018 14:07:41 EEST Linus Walleij wrote: >> On Wed, Sep 5, 2018 at 8:32 PM Stefan Agner wrote: >> > On 05.09.2018 00:44, Laurent Pinchart wrote: >> > >> > Good point! I actually really don't like that we use the same flags here >> > but from a different perspective. Especially since the flags defines >> > document things differently: >> > >> > /* drive data on pos. edge */ >> > #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) >> > /* drive data on neg. edge */ >> > #define DRM_BUS_FLAG_PIXDATA_NEGEDGE(1<<3) >> >> Maybe a stupid comment from my side, but can't we just change the >> documentation to match the usecases? >> >> /* Trigger pixel data latch on positive edge */ >> #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) > > The flags are used for the drm_connector bus_flags field, and really mean > driving on the positive/negative edges. We this can't change their meaning > like this. > >> > Using the opposite perspective would also need translation in crtc >> > drivers... So far no driver uses sampling_edge. >> > >> > I would prefer if we always use the meaning as documented by the flags. >> > >> > I guess we would need to convert DRM_BUS_FLAG_PIXDATA_POSEDGE -> >> > DRM_BUS_FLAG_PIXDATA_NEGEDGE. >> > >> > Linus Walleij, you added sampling edge, any thoughts? >> >> I just thought it was generally useful to have triggering edge encoded >> into the bridge as it makes it clear that this edge is something >> that is a delayed version of the driving edge which is subject to >> clock skew caused by the speed of electrons in silicon and >> copper and slew rate caused by parasitic capacitance. > > I agree that we need both the driving and sampling edge. In many case they > will be opposite, and providing some kind of appropriate defaults in APIs is > fine by me, but we need a way to specify both when needed. We do have pixel clock flags for displays, but also they are actually controller oriented: https://elixir.bootlin.com/linux/latest/source/include/video/display_timing.h#L15 I guess having different flags to denote driving and sampling edge independently would be ideal. But then, is there really a use case where it wouldn't be the exact opposite? The other bus flags are actually fine as is. I suggest to just stick with the bus flags as we have them now, at least for now. Alternatively, we could provide "consumer/bridge" oriented flags which use the same bit and just are the opposite of the controller oriented flags, e.g.: /* drive data on pos. edge */ #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) /* drive data on neg. edge */ #define DRM_BUS_FLAG_PIXDATA_NEGEDGE(1<<3) /* sample data on neg. edge */ #define DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE (1<<2) /* sample data on pos. edge */ #define DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE (1<<3) -- Stefan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 08/14] drm/amdgpu: Remove default best_encoder hook from DC
> -Original Message-
> From: Daniel Vetter
> Sent: Wednesday, September 5, 2018 9:48 AM
> To: Li, Sun peng (Leo)
> Cc: DRI Development ; Intel Graphics
> Development ; Daniel Vetter
> ; Deucher, Alexander
> ; Wentland, Harry
> ; Grodzovsky, Andrey
> ; Cheng, Tony ; S,
> Shirish
> Subject: Re: [PATCH 08/14] drm/amdgpu: Remove default best_encoder
> hook from DC
>
> On Wed, Sep 5, 2018 at 3:45 PM, Leo Li wrote:
> >
> >
> > On 2018-09-03 12:54 PM, Daniel Vetter wrote:
> >>
> >> For atomic driver this is the default, no need to reimplement it. We
> >> still need to keep the copypasta for not-atomic drivers though, since
> >> no one polished the legacy crtc helpers as much as the atomic ones.
> >
> >
> > Thanks for the patch! It seems I made an identical change here:
> > https://lists.freedesktop.org/archives/amd-gfx/2018-August/026064.html
> >
> > One line difference though. I wasn't aware that the default is used
> > when .best_encoder is null, so I just hooked it to the default helper.
> >
> > If it's OK with you, I'll do a follow-up to drop the hook, so we don't
> > have two conflicting patches in flight.
>
> I have a follow-up patches to unexport the helper, so would be good if Alex
> sends out the pull request so I can sort this all out in drm-misc-next. I
> don't
> want to split up the patch series if possible.
> Or we just deal with the conflict, it's minor.
>
> Alex?
I'm planning to send the PR next week when I'm back in the office. I can drop
Leo's patch when I send the PR if that makes things easier.
Alex
> -Daniel
>
> > Leo
> >
> >
> >>
> >> Signed-off-by: Daniel Vetter
> >> Cc: Alex Deucher
> >> Cc: Harry Wentland
> >> Cc: Andrey Grodzovsky
> >> Cc: Tony Cheng
> >> Cc: "Leo (Sunpeng) Li"
> >> Cc: Shirish S
> >> ---
> >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23
> ---
> >> 1 file changed, 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> index af6adffba788..333f9904f135 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -2794,28 +2794,6 @@ static const struct drm_connector_funcs
> >> amdgpu_dm_connector_funcs = {
> >> .atomic_get_property =
> amdgpu_dm_connector_atomic_get_property
> >> };
> >> -static struct drm_encoder *best_encoder(struct drm_connector
> >> *connector)
> >> -{
> >> - int enc_id = connector->encoder_ids[0];
> >> - struct drm_mode_object *obj;
> >> - struct drm_encoder *encoder;
> >> -
> >> - DRM_DEBUG_DRIVER("Finding the best encoder\n");
> >> -
> >> - /* pick the encoder ids */
> >> - if (enc_id) {
> >> - obj = drm_mode_object_find(connector->dev, NULL, enc_id,
> >> DRM_MODE_OBJECT_ENCODER);
> >> - if (!obj) {
> >> - DRM_ERROR("Couldn't find a matching encoder for
> >> our connector\n");
> >> - return NULL;
> >> - }
> >> - encoder = obj_to_encoder(obj);
> >> - return encoder;
> >> - }
> >> - DRM_ERROR("No encoder id\n");
> >> - return NULL;
> >> -}
> >> -
> >> static int get_modes(struct drm_connector *connector)
> >> {
> >> return amdgpu_dm_connector_get_modes(connector);
> >> @@ -2934,7 +2912,6 @@ amdgpu_dm_connector_helper_funcs = {
> >> */
> >> .get_modes = get_modes,
> >> .mode_valid = amdgpu_dm_connector_mode_valid,
> >> - .best_encoder = best_encoder
> >> };
> >> static void dm_crtc_helper_disable(struct drm_crtc *crtc)
> >>
> >
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107784] [AMD tahiti XT] displayport broken
https://bugs.freedesktop.org/show_bug.cgi?id=107784 --- Comment #17 from Sylvain BERTRAND --- On Thu, Sep 06, 2018 at 02:22:18PM +, bugzilla-dae...@freedesktop.org wrote: > https://bugs.freedesktop.org/show_bug.cgi?id=107784 > > --- Comment #16 from Michel Dänzer --- > "this commit" being 019cddc88f9e4ae0de2c76802f7137210c2101aa (the I2C merge), > which has two parents. Both of those parent commits contain commit > e2a9ca29b5edc89da2fddeae30e1070b272395c5 (a TSC commit) as part of their > history. So you previously considered commit > e2a9ca29b5edc89da2fddeae30e1070b272395c5 as both bad and good. That's the > inconsistency. > > This most likely means that you're not yet able to reliably determine that a > given commit is bad, e.g. due to not testing (long) enough. Wow! Then it is even worse of what I thought. Due to the violent leap from 4.18 to 4.19, there are zillions of commits, and even nlog(n) bisect will give me ten_s_ of commits to test. Please, could you refine your "long enough" for a blocker pb which happens at boot, once xorg tries to program my displayport screen. That would be based on your experience, something to give me the order of the "long enough". That said, I have a hinch. I am going to setup a much cleaner test env: it's a custom distro which boots in _really_ a few seconds (not in the range of most mainstream distros boot time)-->I am going to slow it down, on purpose (certainly in more than 1 spot). Then, I have an efi framebuffer and I saw some issues about this->I am going to get rid of it. Then, I am not confident in my monitor (see my other bugs), I may use the previous artificial slow down, to power cycle the monitor, before xorg tries to detect and program it. Well, I'll try to figure a way to put my monitor in a "probably" cleaner state (in respect of displayport hotplug support). Oh, and just in case of, I'll stick to the performance cpu governor. If you have any advice about this based on your experience at knowledge , which I cannot match, I'm all eyes and hears. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 107784] [AMD tahiti XT] displayport broken
On Thu, Sep 06, 2018 at 02:22:18PM +, bugzilla-dae...@freedesktop.org wrote: > https://bugs.freedesktop.org/show_bug.cgi?id=107784 > > --- Comment #16 from Michel Dänzer --- > "this commit" being 019cddc88f9e4ae0de2c76802f7137210c2101aa (the I2C merge), > which has two parents. Both of those parent commits contain commit > e2a9ca29b5edc89da2fddeae30e1070b272395c5 (a TSC commit) as part of their > history. So you previously considered commit > e2a9ca29b5edc89da2fddeae30e1070b272395c5 as both bad and good. That's the > inconsistency. > > This most likely means that you're not yet able to reliably determine that a > given commit is bad, e.g. due to not testing (long) enough. Wow! Then it is even worse of what I thought. Due to the violent leap from 4.18 to 4.19, there are zillions of commits, and even nlog(n) bisect will give me ten_s_ of commits to test. Please, could you refine your "long enough" for a blocker pb which happens at boot, once xorg tries to program my displayport screen. That would be based on your experience, something to give me the order of the "long enough". That said, I have a hinch. I am going to setup a much cleaner test env: it's a custom distro which boots in _really_ a few seconds (not in the range of most mainstream distros boot time)-->I am going to slow it down, on purpose (certainly in more than 1 spot). Then, I have an efi framebuffer and I saw some issues about this->I am going to get rid of it. Then, I am not confident in my monitor (see my other bugs), I may use the previous artificial slow down, to power cycle the monitor, before xorg tries to detect and program it. Well, I'll try to figure a way to put my monitor in a "probably" cleaner state (in respect of displayport hotplug support). Oh, and just in case of, I'll stick to the performance cpu governor. If you have any advice about this based on your experience at knowledge , which I cannot match, I'm all eyes and hears. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 97220] miss detect monitor. dell UP3214Q
https://bugs.freedesktop.org/show_bug.cgi?id=97220 --- Comment #7 from Paul Menzel --- (In reply to kenneth johansson from comment #6) > I have no idea what a tiled display is. I believe it means, that there are actually two panels. > If I use the Intel display port I get or more correctly used to get two > displays that I could then combine with xrandr into one larger. Now the > kernel driver presents just the combined one directly no need to manually do > anything. I have the same monitor. With Linux 4.17 and 4.18 at least GDM 3.28 and 3.30 are correctly displayed over the full screen. Could you please retest? PS: The monitor, unfortunately, only gives problems with the free drivers (Intel and AMDGPU) and firmware (UEFI of MSI B350M MORTAR (MS-7A37)). I believe it’s the monitor firmware, but my colleagues say, they do *not* have any problems with the proprietary Nvidia drivers and Microsoft Windows. So, it’s either a bug in the free drivers, or the proprietary drivers work around it somehow. Here are my bug reports for that. I do not know if these are specific drivers bugs or in the generic DRM code. [1]: https://bugs.freedesktop.org/show_bug.cgi?id=107607 [2]: https://bugs.freedesktop.org/show_bug.cgi?id=107845 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 --- Comment #8 from Michel Dänzer --- BTW, any IGT patches containing amdgpu specific code should probably be Cc'd to the amd-gfx mailing list for review. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 1/2] automake: set NM before running the tests
On Thursday, 2018-09-06 15:53:33 +0100, Emil Velikov wrote:
> From: Emil Velikov
>
> Set/export the NM variable since it may not be set already.
>
> Fixes: 4f08bfe96da ("*-symbol-check: Don't hard-code nm executable")
> Cc: Heiko Becker
> Cc: Eric Engestrom
> Signed-off-by: Emil Velikov
> ---
> amdgpu/Makefile.am| 1 +
> etnaviv/Makefile.am | 1 +
> exynos/Makefile.am| 1 +
> freedreno/Makefile.am | 1 +
> intel/Makefile.am | 1 +
> libkms/Makefile.am| 1 +
> nouveau/Makefile.am | 1 +
> omap/Makefile.am | 1 +
> radeon/Makefile.am| 1 +
> tegra/Makefile.am | 1 +
> 10 files changed, 10 insertions(+)
>
> diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
> index a1b0d05c..1a8538f5 100644
> --- a/amdgpu/Makefile.am
> +++ b/amdgpu/Makefile.am
> @@ -47,5 +47,6 @@ libdrm_amdgpuinclude_HEADERS = $(LIBDRM_AMDGPU_H_FILES)
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_amdgpu.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
I thought only double-quotes worked for this?
If this works, series is:
Reviewed-by: Eric Engestrom
> TESTS = amdgpu-symbol-check
> EXTRA_DIST = $(TESTS)
> diff --git a/etnaviv/Makefile.am b/etnaviv/Makefile.am
> index be96ba86..38ed1717 100644
> --- a/etnaviv/Makefile.am
> +++ b/etnaviv/Makefile.am
> @@ -22,5 +22,6 @@ libdrm_etnavivinclude_HEADERS = $(LIBDRM_ETNAVIV_H_FILES)
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_etnaviv.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = etnaviv-symbol-check
> EXTRA_DIST = $(TESTS)
> diff --git a/exynos/Makefile.am b/exynos/Makefile.am
> index f99f8981..c1dda663 100644
> --- a/exynos/Makefile.am
> +++ b/exynos/Makefile.am
> @@ -23,5 +23,6 @@ libdrm_exynosinclude_HEADERS = exynos_drmif.h
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_exynos.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = exynos-symbol-check
> EXTRA_DIST = $(TESTS)
> diff --git a/freedreno/Makefile.am b/freedreno/Makefile.am
> index cbb0d031..f4f0bafe 100644
> --- a/freedreno/Makefile.am
> +++ b/freedreno/Makefile.am
> @@ -27,5 +27,6 @@ libdrm_freedrenocommoninclude_HEADERS =
> $(LIBDRM_FREEDRENO_H_FILES)
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_freedreno.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = freedreno-symbol-check
> EXTRA_DIST = $(TESTS)
> diff --git a/intel/Makefile.am b/intel/Makefile.am
> index c52e8c08..acedb795 100644
> --- a/intel/Makefile.am
> +++ b/intel/Makefile.am
> @@ -56,6 +56,7 @@ BATCHES = \
> tests/gen7-2d-copy.batch \
> tests/gen7-3d.batch
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = \
> $(BATCHES:.batch=.batch.sh) \
> intel-symbol-check
> diff --git a/libkms/Makefile.am b/libkms/Makefile.am
> index 461fc35b..cd273fa7 100644
> --- a/libkms/Makefile.am
> +++ b/libkms/Makefile.am
> @@ -39,5 +39,6 @@ libkmsinclude_HEADERS = $(LIBKMS_H_FILES)
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libkms.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = kms-symbol-check
> EXTRA_DIST = $(TESTS)
> diff --git a/nouveau/Makefile.am b/nouveau/Makefile.am
> index 344a8445..9f61491b 100644
> --- a/nouveau/Makefile.am
> +++ b/nouveau/Makefile.am
> @@ -29,5 +29,6 @@ libdrm_nouveaunvifinclude_HEADERS = nvif/class.h \
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_nouveau.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = nouveau-symbol-check
> EXTRA_DIST = $(TESTS)
> diff --git a/omap/Makefile.am b/omap/Makefile.am
> index 599bb9de..56257c89 100644
> --- a/omap/Makefile.am
> +++ b/omap/Makefile.am
> @@ -20,5 +20,6 @@ libdrm_omapinclude_HEADERS = omap_drmif.h
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_omap.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = omap-symbol-check
> EXTRA_DIST = $(TESTS)
> diff --git a/radeon/Makefile.am b/radeon/Makefile.am
> index e2415314..0f5f94a1 100644
> --- a/radeon/Makefile.am
> +++ b/radeon/Makefile.am
> @@ -43,5 +43,6 @@ libdrm_radeoninclude_HEADERS = $(LIBDRM_RADEON_H_FILES)
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_radeon.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = radeon-symbol-check
> EXTRA_DIST = $(LIBDRM_RADEON_BOF_FILES) $(TESTS)
> diff --git a/tegra/Makefile.am b/tegra/Makefile.am
> index fb40be55..92b2ce2a 100644
> --- a/tegra/Makefile.am
> +++ b/tegra/Makefile.am
> @@ -21,5 +21,6 @@ libdrm_tegrainclude_HEADERS = tegra.h
> pkgconfigdir = @pkgconfigdir@
> pkgconfig_DATA = libdrm_tegra.pc
>
> +AM_TESTS_ENVIRONMENT = NM='$(NM)'
> TESTS = tegra-symbol-check
> EXTRA_DIST = $(TESTS)
> --
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v5 17/19] drm/msm/dpu: remove RM dependency on connector state
On Wed, Sep 05, 2018 at 07:08:26PM -0700, Jeykumar Sankaran wrote:
> Connector states were passed around RM to update the custom
> topology connector property with chosen topology data. Now that
> we got rid of both custom properties and topology names, this
> change cleans up the mechanism to pass connector states across
> RM helpers and encoder functions.
>
> changes in v5:
> - Introduced in the series
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 15 +++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 3 +-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 3 +-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 60
> +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 -
> 7 files changed, 27 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0d43525..18f5d1d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -436,15 +436,14 @@ int dpu_encoder_helper_unregister_irq(struct
> dpu_encoder_phys *phys_enc,
> }
>
> void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
> - struct dpu_encoder_hw_resources *hw_res,
> - struct drm_connector_state *conn_state)
> + struct dpu_encoder_hw_resources *hw_res)
> {
> struct dpu_encoder_virt *dpu_enc = NULL;
> int i = 0;
>
> - if (!hw_res || !drm_enc || !conn_state) {
> - DPU_ERROR("invalid argument(s), drm_enc %d, res %d, state %d\n",
> - drm_enc != 0, hw_res != 0, conn_state != 0);
> + if (!hw_res || !drm_enc) {
> + DPU_ERROR("invalid argument(s), drm_enc %d, res %d\n",
> + drm_enc != 0, hw_res != 0);
As far as I can tell both hw_res and drm_enc will be set for all the callers so
this check shouldn't be needed. If you are paranoid you can switch to a
WARN_ON() and get rid of the custom error message but I don't think you need to.
> return;
> }
>
> @@ -458,7 +457,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder
> *drm_enc,
> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>
> if (phys && phys->ops.get_hw_resources)
> - phys->ops.get_hw_resources(phys, hw_res, conn_state);
> + phys->ops.get_hw_resources(phys, hw_res);
> }
> }
>
> @@ -652,7 +651,7 @@ static int dpu_encoder_virt_atomic_check(
> if (drm_atomic_crtc_needs_modeset(crtc_state)
> && dpu_enc->mode_set_complete) {
> ret = dpu_rm_reserve(_kms->rm, drm_enc, crtc_state,
> - conn_state, topology, true);
> + topology, true);
> dpu_enc->mode_set_complete = false;
> }
> }
> @@ -1044,7 +1043,7 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
>
> /* Reserve dynamic resources now. Indicating non-AtomicTest phase */
> ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_enc->crtc->state,
> - conn->state, topology, false);
> + topology, false);
> if (ret) {
> DPU_ERROR_ENC(dpu_enc,
> "failed to reserve hw resources, %d\n", ret);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index f109b4d..34ac5b6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -51,11 +51,9 @@ struct dpu_encoder_kickoff_params {
> * dpu_encoder_get_hw_resources - Populate table of required hardware
> resources
> * @encoder: encoder pointer
> * @hw_res: resource table to populate with encoder required resources
> - * @conn_state: report hw reqs based on this proposed connector state
> */
> void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
> - struct dpu_encoder_hw_resources *hw_res,
> - struct drm_connector_state *conn_state);
> + struct dpu_encoder_hw_resources *hw_res);
>
> /**
> * dpu_encoder_register_vblank_callback - provide callback to encoder that
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index a00b222..3fe4ed9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -140,8 +140,7 @@ struct dpu_encoder_phys_ops {
> struct drm_connector_state *conn_state);
> void (*destroy)(struct
Re: [PATCH libdrm] gitlab-ci: use variables to deduplicate the build commands
On Thursday, 2018-09-06 16:01:15 +0100, Emil Velikov wrote: > On 6 September 2018 at 14:40, Eric Engestrom wrote: > > Signed-off-by: Eric Engestrom > > --- > > .gitlab-ci.yml | 129 ++--- > > 1 file changed, 47 insertions(+), 82 deletions(-) > > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > index eee6abfcdd7de2839660..1dc434a5d359b3b077e7 100644 > > --- a/.gitlab-ci.yml > > +++ b/.gitlab-ci.yml > > @@ -1,3 +1,46 @@ > > +.meson-build: > > Gitlab calls these templates, not variables. With that fixed > Reviewed-by: Emil Velikov Thanks, I should've looked that up :] Fixed and pushed. > > -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [RFC v2] Drop all 00-INDEX files from Documentation/
On Thu, 6 Sep 2018 09:58:04 -0600 Jonathan Corbet wrote: > Thanks, > > jon (who is increasingly inclined to apply this patch) As Colin Kaepernick now says... "Just do it!" ;-) -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [RFC v2] Drop all 00-INDEX files from Documentation/
On Tue, 4 Sep 2018 09:59:08 -0400 Steven Rostedt wrote: > On Tue, 4 Sep 2018 13:30:30 +0200 > Pavel Machek wrote: > > > I'd say this is still quite valueable, and it might be worth fixing, > > rather then removing completely. > > I agree. Perhaps we should have a 00-DESCRIPTION file in each > directory, and each file could start with a: > > DESCRIPTION: > > and then these files could be generated by those that have these tags. I really don't want to hack up yet another documentation syntax and processing scheme. We already have one that does all of this and more. That energy would be far better spent bringing the docs into the RST hierarchy, IMO. Thanks, jon (who is increasingly inclined to apply this patch) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 --- Comment #7 from Michel Dänzer --- (In reply to Martin Peres from comment #4) > However, if it is scheduler bug, I would assume this issue to be > reproducible on any AMD GPU. Can you try running > igt@amdgpu/amd_cs_nop@sync-fork-gfx0 in a loop for an hour or so? Hmm, 'fork' sounded suspicious, and indeed AFAICT this test uses the same amdgpu_context_handle (and by extension the same DRM file descriptor) in multiple processes spawned by fork(). I'm afraid Christian is going to explain why that's not supported. :) I suppose we should at least handle this more gracefully though, if possible. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 01/11] symbols-check: add new meta-script
On 4 April 2018 at 16:41, Eric Engestrom wrote: > --- /dev/null > +++ b/symbols-check > @@ -0,0 +1,79 @@ > +#!/bin/sh > +set -eu > +set -o pipefail > + We could drop the execute bit, shebang and set. Pretty much all of it is handled by the callers (modulo pipefail) > +if [ -z "$LIB" ]; then > + echo "\$LIB needs to be defined for autotools to be able to run this test" > + exit 1 > +fi > + > +# The lib name is passed in with Meson but autotools doesn't support that > +# so it needs to be hardcoded and overwritten here > +if [ $# -ge 1 ]; then > + LIB=$1 > +fi > + > +if ! [ -f "$LIB" ]; then > + echo "lib $LIB doesn't exist" > + exit 1 > +fi > + > +if [ -z "$NM" ]; then > + echo "\$NM is undefined or empty" > + exit 1 I would drop all these - set -u will provide reasonable error handling HTH Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 02/11] amdgpu: use new symbols checking script
On 4 April 2018 at 16:41, Eric Engestrom wrote: > Signed-off-by: Eric Engestrom > --- > amdgpu/Makefile.am | 1 + > amdgpu/amdgpu-symbol-check | 19 ++- > 2 files changed, 7 insertions(+), 13 deletions(-) > > diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am > index a1b0d05c1457ae681ac8..4ad6949e81ddbd11078e 100644 > --- a/amdgpu/Makefile.am > +++ b/amdgpu/Makefile.am > @@ -47,5 +47,6 @@ libdrm_amdgpuinclude_HEADERS = $(LIBDRM_AMDGPU_H_FILES) > pkgconfigdir = @pkgconfigdir@ > pkgconfig_DATA = libdrm_amdgpu.pc > > +AM_TESTS_ENVIRONMENT = top_srcdir='$(top_srcdir)' Gut suggests that the absolute abs_foo path will be needed here - not too sure though. > diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check > index 90b7a1d633c0b2143f29..5ed24b906c0228233f71 100755 > --- a/amdgpu/amdgpu-symbol-check > +++ b/amdgpu/amdgpu-symbol-check > @@ -1,15 +1,10 @@ > #!/bin/bash > +set -eu > > -# The following symbols (past the first five) are taken from the public > headers. > -# A list of the latter should be available > Makefile.am/libdrm_amdgpuinclude_HEADERS > +LIB=.libs/libdrm_amdgpu.so > I think this will break meson - it passes the library name as an arg. One solution is to set LIB (feel free to pick better name), just like NM. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] CI: Capture test logs as GitLab artifacts
On Thu, Sep 06, 2018 at 12:38:46PM +0200, Daniel Vetter wrote: > On Thu, Sep 06, 2018 at 11:01:17AM +0100, Daniel Stone wrote: > > GitLab CI already captures all the stdout/stderr output from the build > > process as the log. However, some other important information is hidden > > in other log files. > > > > Taken from Wayland, capture logs from the configuration process as well > > as from every check. > > > > Signed-off-by: Daniel Stone > > Cc: Rodrigo Vivi > > Cc: Lucas De Marchi > > Cc: Eric Engeström > > Cc: Daniel Vetter > > I think there's also an option to capture all non-tracked files when the > build fails. For the lazy :-) But this looks good too. One person's lazy is another person's efficient [1] :-) Sean [1]- https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/commit/68a78ef6bd8b8ddf710e9a0654fa47cdc1f158d3#587d266bb27a4dc3022bbed44dfa19849df3044c_17_15 > > Acked-by: Daniel Vetter > > --- > > .gitlab-ci.yml | 26 ++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > index eee6abfc..50ec8527 100644 > > --- a/.gitlab-ci.yml > > +++ b/.gitlab-ci.yml > > @@ -1,6 +1,29 @@ > > +.artifacts-meson: > > + when: always > > + paths: > > + - _build/meson-logs > > + > > +.artifacts-autotools: > > + when: always > > + paths: > > +- _build/*.log > > +- _build/amdgpu/*.log > > +- _build/etnaviv/*.log > > +- _build/exynos/*.log > > +- _build/freedreno/*.log > > +- _build/intel/*.log > > +- _build/libkms/*.log > > +- _build/nouveau/*.log > > +- _build/omap/*.log > > +- _build/radeon/*.log > > +- _build/tegra/*.log > > +- _build/tests/*.log > > +- _build/tests/*/*.log > > + > > latest-meson: > >stage: build > >image: base/archlinux:latest > > + artifacts: *artifacts-meson > >before_script: > > - pacman -Syu --noconfirm --needed > > base-devel > > @@ -35,6 +58,7 @@ latest-meson: > > latest-autotools: > >stage: build > >image: base/archlinux:latest > > + artifacts: *artifacts-autotools > >before_script: > > - pacman -Syu --noconfirm --needed > > base-devel > > @@ -69,6 +93,7 @@ latest-autotools: > > oldest-meson: > >stage: build > >image: debian:stable > > + artifacts: *artifacts-meson > >before_script: > > - printf > /etc/dpkg/dpkg.cfg.d/99-exclude-cruft "%s\n" > > 'path-exclude=/usr/share/doc/*' > > @@ -125,6 +150,7 @@ oldest-meson: > > oldest-autotools: > >stage: build > >image: debian:stable > > + artifacts: *artifacts-autotools > >before_script: > > - printf > /etc/dpkg/dpkg.cfg.d/99-exclude-cruft "%s\n" > > 'path-exclude=/usr/share/doc/*' > > -- > > 2.19.0.rc0 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107781] amdgpu hangs on resume on Lenovo A475
https://bugs.freedesktop.org/show_bug.cgi?id=107781 Alex Findler changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #9 from Alex Findler --- I've suspended by closing the lid, pushing the power button, using the Plasma option, docked, undocked, docked while suspended, and it all works quite well with 4.18.5. Thank you very much for this quick resolution, thanks for the good work. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 --- Comment #6 from Chris Wilson --- Since it just happened again with drm-tip 60edf94611d2 that includes 4823e5da2e https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4782/fi-kbl-8809g/igt@amdgpu_amd_cs_...@sync-fork-gfx0.html you are in luck. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 --- Comment #5 from Martin Peres --- (In reply to Lucas Stach from comment #3) > We've just fixed a bug in the scheduler timeout handling, which might lead > to the timeout worker being armed for the wrong job. > > Does this issue still occur on a recent kernel with > 4823e5da2ea9061011242db81334d6ebbd2ed0a5 (drm/scheduler: fix timeout worker > setup for out of order job completions) present? I see! We are using drmtip, which does not yet include 4823e5da2ea9061011242db81334d6ebbd2ed0a5. I assume we'll get it very soon, but given the reproduction rate of the issue, we will not know for sure if the issue is fixed for at least another month (it happened twice in a week, 2% of our runs). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 --- Comment #4 from Martin Peres --- (In reply to Michel Dänzer from comment #2) > (In reply to Martin Peres from comment #0) > > [ 358.292609] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 > > timeout, signaled seq=137, emitted seq=137 > > [ 358.292635] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma1 > > timeout, signaled seq=145, emitted seq=145 > > (In reply to Martin Peres from comment #1) > > [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled > > seq=137, emitted seq=137 > > [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled > > seq=147, emitted seq=147 > > Hmm, signalled and emitted sequence numbers are always the same, meaning the > hardware hasn't actually timed out? > > I can think of two possibilities: > > * A GPU scheduler bug causing the job timeout handling to be triggered > spuriously. (Could something be stalling the system work queue, so the items > scheduled by drm_sched_job_finish_cb can't call drm_sched_job_finish in > time?) > > * A problem with the handling of the GPU's interrupts. Do the numbers on the > amdgpu line in /proc/interrupts still increase after these messages > appeared, or at least in the ten seconds before they appear? Here is the IGT run log: [283/301] skip: 65, pass: 218 - running: igt/amdgpu/amd_cs_nop/sync-fork-gfx0 [283/301] skip: 65, pass: 218 \ dmesg-warn: igt/amdgpu/amd_cs_nop/sync-fork-gfx0 [284/301] skip: 65, pass: 218, dmesg-warn: 1 \ running: igt/amdgpu/amd_prime/i915-to-amd [284/301] skip: 65, pass: 218, dmesg-warn: 1 | pass: igt/amdgpu/amd_prime/i915-to-amd [285/301] skip: 65, pass: 219, dmesg-warn: 1 | running: igt/amdgpu/amd_prime/amd-to-i915 [285/301] skip: 65, pass: 219, dmesg-warn: 1 / dmesg-fail: igt/amdgpu/amd_prime/amd-to-i915 It shows that both the tests #283 and #285 generated the timeout, yet the seqno has increased by 10 between the two tests, suggesting that the GPU is not hung. I can't easily check if interrupts in /proc/interrupts are still increasing on a machine that is part of our CI, but I guess if this is what it takes to get this bug forward, I will try to get my hands on a KBLg platform and help you trigger. However, if it is scheduler bug, I would assume this issue to be reproducible on any AMD GPU. Can you try running igt@amdgpu/amd_cs_nop@sync-fork-gfx0 in a loop for an hour or so? Your second proposal would point at a KBLg-specific bug, but let's first rule out the scheduler as being part of the problem. In any case, thanks for your answer :) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 --- Comment #3 from Lucas Stach --- We've just fixed a bug in the scheduler timeout handling, which might lead to the timeout worker being armed for the wrong job. Does this issue still occur on a recent kernel with 4823e5da2ea9061011242db81334d6ebbd2ed0a5 (drm/scheduler: fix timeout worker setup for out of order job completions) present? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] intel: annotate the intel genx helpers as private
From: Emil Velikov
They're used internally and never meant to be part of the API.
Add the drm_private notation, which should resolve that.
Cc: Eric Engestrom
Cc: Lucas De Marchi
Cc: Chris Wilson
Cc: Rodrigo Vivi
Fixes: 4e81d4f9c9b ("intel: add generic functions to check PCI ID")
Signed-off-by: Emil Velikov
---
intel/intel_chipset.c | 4 ++--
intel/intel_chipset.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
index d5c33cc5..5aa4a2f2 100644
--- a/intel/intel_chipset.c
+++ b/intel/intel_chipset.c
@@ -44,7 +44,7 @@ static const struct pci_device {
INTEL_SKL_IDS(9),
};
-bool intel_is_genx(unsigned int devid, int gen)
+drm_private bool intel_is_genx(unsigned int devid, int gen)
{
const struct pci_device *p,
*pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
@@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
return false;
}
-bool intel_get_genx(unsigned int devid, int *gen)
+drm_private bool intel_get_genx(unsigned int devid, int *gen)
{
const struct pci_device *p,
*pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
index 9b1e64f1..63ddde7a 100644
--- a/intel/intel_chipset.h
+++ b/intel/intel_chipset.h
@@ -330,8 +330,8 @@
/* New platforms use kernel pci ids */
#include
-bool intel_is_genx(unsigned int devid, int gen);
-bool intel_get_genx(unsigned int devid, int *gen);
+drm_private bool intel_is_genx(unsigned int devid, int gen);
+drm_private bool intel_get_genx(unsigned int devid, int *gen);
#define IS_GEN9(devid) intel_is_genx(devid, 9)
#define IS_GEN10(devid) intel_is_genx(devid, 10)
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 Michel Dänzer changed: What|Removed |Added CC||ckoenig.leichtzumerken@gmai ||l.com, d...@lynxeye.de --- Comment #2 from Michel Dänzer --- (In reply to Martin Peres from comment #0) > [ 358.292609] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, > signaled seq=137, emitted seq=137 > [ 358.292635] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma1 timeout, > signaled seq=145, emitted seq=145 (In reply to Martin Peres from comment #1) > [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled > seq=137, emitted seq=137 > [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled > seq=147, emitted seq=147 Hmm, signalled and emitted sequence numbers are always the same, meaning the hardware hasn't actually timed out? I can think of two possibilities: * A GPU scheduler bug causing the job timeout handling to be triggered spuriously. (Could something be stalling the system work queue, so the items scheduled by drm_sched_job_finish_cb can't call drm_sched_job_finish in time?) * A problem with the handling of the GPU's interrupts. Do the numbers on the amdgpu line in /proc/interrupts still increase after these messages appeared, or at least in the ten seconds before they appear? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm | Pipeline #4114 has failed for master | c55f1b9b
HI all, On 6 September 2018 at 07:10, Lucas De Marchi wrote: > On Wed, Sep 5, 2018 at 7:00 PM Rodrigo Vivi wrote: >> >> well it builds for me. >> >> but any idea what might be wrong here on gitlab ci? > FWIW gitlab gives you the test target and command used. It might be a bit hard to find at first though :-\ FAILED: meson-test /usr/bin/python3 /usr/local/bin/meson test --no-rebuild --print-errorlogs > From the logs: > > The output from the failed tests: > > 8/21 intel-symbol-check FAIL 0.29 s (exit status 1) > > --- command --- > NM='/usr/sbin/nm' /usr/sbin/bash > /builds/mesa/drm/_build/../intel/intel-symbol-check > intel/libdrm_intel.so.1.0.0 > --- stdout --- > intel_get_genx intel_is_genx > --- > > So... it's not the build that fails, the test is checking for exported > symbols, and it only does that when using meson (actually as part of > the tests). And we don't use -fvisibility=hidden?!??! Ugh, I think we > need some fixing on that :( > I nuked it back in 0f8da82500ec542e269092c0718479e25eaff5f6 because weird compilers. I guess we can reconsider that? On the other hand: git grep does point out to drm_private. Will send a patch in a moment, unless someone beats me to it. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] gitlab-ci: use variables to deduplicate the build commands
On 6 September 2018 at 14:40, Eric Engestrom wrote: > Signed-off-by: Eric Engestrom > --- > .gitlab-ci.yml | 129 ++--- > 1 file changed, 47 insertions(+), 82 deletions(-) > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index eee6abfcdd7de2839660..1dc434a5d359b3b077e7 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -1,3 +1,46 @@ > +.meson-build: Gitlab calls these templates, not variables. With that fixed Reviewed-by: Emil Velikov -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/2] *-symbols-check: error out when using unset variables
From: Emil Velikov
It will make bugs like the one fixed with previous patch dead obvious.
Cc: Eric Engestrom
Signed-off-by: Emil Velikov
---
amdgpu/amdgpu-symbol-check | 2 ++
etnaviv/etnaviv-symbol-check | 2 ++
exynos/exynos-symbol-check | 2 ++
freedreno/freedreno-symbol-check | 2 ++
intel/intel-symbol-check | 2 ++
libkms/kms-symbol-check | 2 ++
nouveau/nouveau-symbol-check | 2 ++
omap/omap-symbol-check | 2 ++
radeon/radeon-symbol-check | 2 ++
tegra/tegra-symbol-check | 2 ++
10 files changed, 20 insertions(+)
diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
index 90b7a1d6..07477243 100755
--- a/amdgpu/amdgpu-symbol-check
+++ b/amdgpu/amdgpu-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available
Makefile.am/libdrm_amdgpuinclude_HEADERS
diff --git a/etnaviv/etnaviv-symbol-check b/etnaviv/etnaviv-symbol-check
index bc509615..18910688 100755
--- a/etnaviv/etnaviv-symbol-check
+++ b/etnaviv/etnaviv-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available
Makefile.sources/LIBDRM_ETNAVIV_H_FILES
diff --git a/exynos/exynos-symbol-check b/exynos/exynos-symbol-check
index e9f1b04d..49d611e6 100755
--- a/exynos/exynos-symbol-check
+++ b/exynos/exynos-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available Makefile.am/libdrm_exynos*_HEADERS
diff --git a/freedreno/freedreno-symbol-check b/freedreno/freedreno-symbol-check
index e732c995..6da9d667 100755
--- a/freedreno/freedreno-symbol-check
+++ b/freedreno/freedreno-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available
Makefile.sources/LIBDRM_FREEDRENO_H_FILES
diff --git a/intel/intel-symbol-check b/intel/intel-symbol-check
index 4d30a4b1..de377bef 100755
--- a/intel/intel-symbol-check
+++ b/intel/intel-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available
Makefile.sources/LIBDRM_INTEL_H_FILES
diff --git a/libkms/kms-symbol-check b/libkms/kms-symbol-check
index a5c2120d..30f444f7 100755
--- a/libkms/kms-symbol-check
+++ b/libkms/kms-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available Makefile.sources/LIBKMS_H_FILES
diff --git a/nouveau/nouveau-symbol-check b/nouveau/nouveau-symbol-check
index b3a24101..6296244c 100755
--- a/nouveau/nouveau-symbol-check
+++ b/nouveau/nouveau-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available
Makefile.sources/LIBDRM_NOUVEAU_H_FILES
diff --git a/omap/omap-symbol-check b/omap/omap-symbol-check
index 0fb4a0f2..16da3c40 100755
--- a/omap/omap-symbol-check
+++ b/omap/omap-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available Makefile.am/libdrm_omap*HEADERS
diff --git a/radeon/radeon-symbol-check b/radeon/radeon-symbol-check
index 7d79d901..da605bb8 100755
--- a/radeon/radeon-symbol-check
+++ b/radeon/radeon-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first five) are taken from the public
headers.
# A list of the latter should be available
Makefile.sources/LIBDRM_RADEON_H_FILES
diff --git a/tegra/tegra-symbol-check b/tegra/tegra-symbol-check
index 509b678c..8539b95b 100755
--- a/tegra/tegra-symbol-check
+++ b/tegra/tegra-symbol-check
@@ -1,5 +1,7 @@
#!/bin/bash
+set -u
+
# The following symbols (past the first nine) are taken from tegra.h.
FUNCS=$($NM -D --format=bsd --defined-only ${1-.libs/libdrm_tegra.so} | awk
'{print $3}'| while read func; do
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/2] automake: set NM before running the tests
From: Emil Velikov
Set/export the NM variable since it may not be set already.
Fixes: 4f08bfe96da ("*-symbol-check: Don't hard-code nm executable")
Cc: Heiko Becker
Cc: Eric Engestrom
Signed-off-by: Emil Velikov
---
amdgpu/Makefile.am| 1 +
etnaviv/Makefile.am | 1 +
exynos/Makefile.am| 1 +
freedreno/Makefile.am | 1 +
intel/Makefile.am | 1 +
libkms/Makefile.am| 1 +
nouveau/Makefile.am | 1 +
omap/Makefile.am | 1 +
radeon/Makefile.am| 1 +
tegra/Makefile.am | 1 +
10 files changed, 10 insertions(+)
diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
index a1b0d05c..1a8538f5 100644
--- a/amdgpu/Makefile.am
+++ b/amdgpu/Makefile.am
@@ -47,5 +47,6 @@ libdrm_amdgpuinclude_HEADERS = $(LIBDRM_AMDGPU_H_FILES)
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_amdgpu.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = amdgpu-symbol-check
EXTRA_DIST = $(TESTS)
diff --git a/etnaviv/Makefile.am b/etnaviv/Makefile.am
index be96ba86..38ed1717 100644
--- a/etnaviv/Makefile.am
+++ b/etnaviv/Makefile.am
@@ -22,5 +22,6 @@ libdrm_etnavivinclude_HEADERS = $(LIBDRM_ETNAVIV_H_FILES)
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_etnaviv.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = etnaviv-symbol-check
EXTRA_DIST = $(TESTS)
diff --git a/exynos/Makefile.am b/exynos/Makefile.am
index f99f8981..c1dda663 100644
--- a/exynos/Makefile.am
+++ b/exynos/Makefile.am
@@ -23,5 +23,6 @@ libdrm_exynosinclude_HEADERS = exynos_drmif.h
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_exynos.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = exynos-symbol-check
EXTRA_DIST = $(TESTS)
diff --git a/freedreno/Makefile.am b/freedreno/Makefile.am
index cbb0d031..f4f0bafe 100644
--- a/freedreno/Makefile.am
+++ b/freedreno/Makefile.am
@@ -27,5 +27,6 @@ libdrm_freedrenocommoninclude_HEADERS =
$(LIBDRM_FREEDRENO_H_FILES)
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_freedreno.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = freedreno-symbol-check
EXTRA_DIST = $(TESTS)
diff --git a/intel/Makefile.am b/intel/Makefile.am
index c52e8c08..acedb795 100644
--- a/intel/Makefile.am
+++ b/intel/Makefile.am
@@ -56,6 +56,7 @@ BATCHES = \
tests/gen7-2d-copy.batch \
tests/gen7-3d.batch
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = \
$(BATCHES:.batch=.batch.sh) \
intel-symbol-check
diff --git a/libkms/Makefile.am b/libkms/Makefile.am
index 461fc35b..cd273fa7 100644
--- a/libkms/Makefile.am
+++ b/libkms/Makefile.am
@@ -39,5 +39,6 @@ libkmsinclude_HEADERS = $(LIBKMS_H_FILES)
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libkms.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = kms-symbol-check
EXTRA_DIST = $(TESTS)
diff --git a/nouveau/Makefile.am b/nouveau/Makefile.am
index 344a8445..9f61491b 100644
--- a/nouveau/Makefile.am
+++ b/nouveau/Makefile.am
@@ -29,5 +29,6 @@ libdrm_nouveaunvifinclude_HEADERS = nvif/class.h \
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_nouveau.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = nouveau-symbol-check
EXTRA_DIST = $(TESTS)
diff --git a/omap/Makefile.am b/omap/Makefile.am
index 599bb9de..56257c89 100644
--- a/omap/Makefile.am
+++ b/omap/Makefile.am
@@ -20,5 +20,6 @@ libdrm_omapinclude_HEADERS = omap_drmif.h
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_omap.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = omap-symbol-check
EXTRA_DIST = $(TESTS)
diff --git a/radeon/Makefile.am b/radeon/Makefile.am
index e2415314..0f5f94a1 100644
--- a/radeon/Makefile.am
+++ b/radeon/Makefile.am
@@ -43,5 +43,6 @@ libdrm_radeoninclude_HEADERS = $(LIBDRM_RADEON_H_FILES)
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_radeon.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = radeon-symbol-check
EXTRA_DIST = $(LIBDRM_RADEON_BOF_FILES) $(TESTS)
diff --git a/tegra/Makefile.am b/tegra/Makefile.am
index fb40be55..92b2ce2a 100644
--- a/tegra/Makefile.am
+++ b/tegra/Makefile.am
@@ -21,5 +21,6 @@ libdrm_tegrainclude_HEADERS = tegra.h
pkgconfigdir = @pkgconfigdir@
pkgconfig_DATA = libdrm_tegra.pc
+AM_TESTS_ENVIRONMENT = NM='$(NM)'
TESTS = tegra-symbol-check
EXTRA_DIST = $(TESTS)
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/10] phy: Add configuration interface
Hi Kishon,
On Thu, Sep 06, 2018 at 02:57:58PM +0530, Kishon Vijay Abraham I wrote:
> On Wednesday 05 September 2018 02:46 PM, Maxime Ripard wrote:
> > The phy framework is only allowing to configure the power state of the PHY
> > using the init and power_on hooks, and their power_off and exit
> > counterparts.
> >
> > While it works for most, simple, PHYs supported so far, some more advanced
> > PHYs need some configuration depending on runtime parameters. These PHYs
> > have been supported by a number of means already, often by using ad-hoc
> > drivers in their consumer drivers.
> >
> > That doesn't work too well however, when a consumer device needs to deal
> > multiple PHYs, or when multiple consumers need to deal with the same PHY (a
> > DSI driver and a CSI driver for example).
> >
> > So we'll add a new interface, through two funtions, phy_validate and
> > phy_configure. The first one will allow to check that a current
> > configuration, for a given mode, is applicable. It will also allow the PHY
> > driver to tune the settings given as parameters as it sees fit.
> >
> > phy_configure will actually apply that configuration in the phy itself.
> >
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/phy/phy-core.c | 62 ++-
> > include/linux/phy/phy.h | 42 -
> > 2 files changed, 104 insertions(+)
> >
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 35fd38c5a4a1..6eaf655e370f 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -408,6 +408,68 @@ int phy_calibrate(struct phy *phy)
> > EXPORT_SYMBOL_GPL(phy_calibrate);
> >
> > /**
> > + * phy_configure() - Changes the phy parameters
> > + * @phy: the phy returned by phy_get()
> > + * @mode: phy_mode the configuration is applicable to.
>
> mode should be used if the same PHY can be configured in multiple modes. But
> with phy_set_mode() and phy_calibrate() we could achieve the same.
So you would change the prototype to have a configuration applying
only to the current mode set previously through set_mode?
Can we have PHY that operate in multiple modes at the same time?
> > + * @opts: New configuration to apply
>
> Should these configuration come from the consumer driver?
Yes
> Can't the helper functions be directly invoked by the PHY driver for
> the configuration.
Not really. The helpers are here to introduce functions that give you
the defaults provided by the spec for a given configuration, and to
validate that a given configuration is within the spec boundaries. I
expect some consumers to need to change the defaults for some more
suited parameters that are still within the boundaries defined by the
spec.
And I'd really want to have that interface being quite generic, and
applicable to other phy modes as well. The allwinner USB PHY for
example require at the moment an extra function that could be moved to
this API:
https://elixir.bootlin.com/linux/latest/source/drivers/phy/allwinner/phy-sun4i-usb.c#L512
> > + *
> > + * Used to change the PHY parameters. phy_init() must have
> > + * been called on the phy.
> > + *
> > + * Returns: 0 if successful, an negative error code otherwise
> > + */
> > +int phy_configure(struct phy *phy, enum phy_mode mode,
> > + union phy_configure_opts *opts)
> > +{> + int ret;
> > +
> > + if (!phy)
> > + return -EINVAL;
> > +
> > + if (!phy->ops->configure)
> > + return 0;
> > +
> > + mutex_lock(>mutex);
> > + ret = phy->ops->configure(phy, mode, opts);
> > + mutex_unlock(>mutex);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * phy_validate() - Checks the phy parameters
> > + * @phy: the phy returned by phy_get()
> > + * @mode: phy_mode the configuration is applicable to.
> > + * @opts: Configuration to check
> > + *
> > + * Used to check that the current set of parameters can be handled by
> > + * the phy. Implementations are free to tune the parameters passed as
> > + * arguments if needed by some implementation detail or
> > + * constraints. It will not change any actual configuration of the
> > + * PHY, so calling it as many times as deemed fit will have no side
> > + * effect.
> > + *
> > + * Returns: 0 if successful, an negative error code otherwise
> > + */
> > +int phy_validate(struct phy *phy, enum phy_mode mode,
> > + union phy_configure_opts *opts)
>
> IIUC the consumer driver will pass configuration options (or PHY parameters)
> which will be validated by the PHY driver and in some cases the PHY driver can
> modify the configuration options? And these modified configuration options
> will
> again be given to phy_configure?
>
> Looks like it's a round about way of doing the same thing.
Not really. The validate callback allows to check whether a particular
configuration would work, and try to negotiate a set of configurations
that both the consumer and the PHY could work with.
For example, DRM requires
Re: [PATCH 02/10] phy: Add configuration interface
On Wed, Sep 05, 2018 at 04:39:46PM +0300, Laurent Pinchart wrote:
> Hi Maxime,
>
> Thank you for the patch.
>
> On Wednesday, 5 September 2018 12:16:33 EEST Maxime Ripard wrote:
> > The phy framework is only allowing to configure the power state of the PHY
> > using the init and power_on hooks, and their power_off and exit
> > counterparts.
> >
> > While it works for most, simple, PHYs supported so far, some more advanced
> > PHYs need some configuration depending on runtime parameters. These PHYs
> > have been supported by a number of means already, often by using ad-hoc
> > drivers in their consumer drivers.
> >
> > That doesn't work too well however, when a consumer device needs to deal
>
> s/deal/deal with/
>
> > multiple PHYs, or when multiple consumers need to deal with the same PHY (a
> > DSI driver and a CSI driver for example).
> >
> > So we'll add a new interface, through two funtions, phy_validate and
> > phy_configure. The first one will allow to check that a current
> > configuration, for a given mode, is applicable. It will also allow the PHY
> > driver to tune the settings given as parameters as it sees fit.
> >
> > phy_configure will actually apply that configuration in the phy itself.
> >
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/phy/phy-core.c | 62 ++-
> > include/linux/phy/phy.h | 42 -
> > 2 files changed, 104 insertions(+)
> >
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 35fd38c5a4a1..6eaf655e370f 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -408,6 +408,68 @@ int phy_calibrate(struct phy *phy)
> > EXPORT_SYMBOL_GPL(phy_calibrate);
> >
> > /**
> > + * phy_configure() - Changes the phy parameters
> > + * @phy: the phy returned by phy_get()
> > + * @mode: phy_mode the configuration is applicable to.
> > + * @opts: New configuration to apply
> > + *
> > + * Used to change the PHY parameters. phy_init() must have
> > + * been called on the phy.
> > + *
> > + * Returns: 0 if successful, an negative error code otherwise
> > + */
> > +int phy_configure(struct phy *phy, enum phy_mode mode,
> > + union phy_configure_opts *opts)
> > +{
> > + int ret;
> > +
> > + if (!phy)
> > + return -EINVAL;
> > +
> > + if (!phy->ops->configure)
> > + return 0;
>
> Shouldn't you report an error to the caller ? If a caller expects the PHY to
> be configurable, I would assume that silently ignoring the requested
> configuration won't work great.
I'm not sure. I also expect a device having to interact with multiple
PHYs, some of them needing some configuration while some other do
not. In that scenario, returning 0 seems to be the right thing to do.
> > + mutex_lock(>mutex);
> > + ret = phy->ops->configure(phy, mode, opts);
> > + mutex_unlock(>mutex);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * phy_validate() - Checks the phy parameters
> > + * @phy: the phy returned by phy_get()
> > + * @mode: phy_mode the configuration is applicable to.
> > + * @opts: Configuration to check
> > + *
> > + * Used to check that the current set of parameters can be handled by
> > + * the phy. Implementations are free to tune the parameters passed as
> > + * arguments if needed by some implementation detail or
> > + * constraints. It will not change any actual configuration of the
> > + * PHY, so calling it as many times as deemed fit will have no side
> > + * effect.
> > + *
> > + * Returns: 0 if successful, an negative error code otherwise
> > + */
> > +int phy_validate(struct phy *phy, enum phy_mode mode,
> > + union phy_configure_opts *opts)
> > +{
> > + int ret;
> > +
> > + if (!phy)
> > + return -EINVAL;
> > +
> > + if (!phy->ops->validate)
> > + return 0;
> > +
> > + mutex_lock(>mutex);
> > + ret = phy->ops->validate(phy, mode, opts);
> > + mutex_unlock(>mutex);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> > * @np: device_node for which to get the phy
> > * @index: the index of the phy
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index 9cba7fe16c23..3cc315dcfcd0 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -44,6 +44,12 @@ enum phy_mode {
> > };
> >
> > /**
> > + * union phy_configure_opts - Opaque generic phy configuration
> > + */
> > +union phy_configure_opts {
> > +};
> > +
> > +/**
> > * struct phy_ops - set of function pointers for performing phy operations
> > * @init: operation to be performed for initializing phy
> > * @exit: operation to be performed while exiting
> > @@ -60,6 +66,38 @@ struct phy_ops {
> > int (*power_on)(struct phy *phy);
> > int (*power_off)(struct phy *phy);
> > int (*set_mode)(struct phy *phy, enum phy_mode mode);
> > +
> > + /**
> > +* @configure:
> > +
Re: [PATCH libdrm] intel: use drm namespace for exported functions
On 6 September 2018 at 14:52, Eric Engestrom wrote: > And add them to the list of exported function to fix the tests. > > Fixes: 4e81d4f9c9b7fd6510cf "intel: add generic functions to check PCI ID" > Cc: Lucas De Marchi > Cc: Rodrigo Vivi > Signed-off-by: Eric Engestrom > --- > Lucas, I'm assuming you meant to export those functions (and macros, > like `IS_GEN11()`), right? > If so, you need to namespace them, and add them to the list of allowed > exports. The function isn't one part of the public API. So I'd keep the name and add the drm_private notation (adds visibility=hidden) -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107762] [Intel GFX CI] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137
https://bugs.freedesktop.org/show_bug.cgi?id=107762 Martin Peres changed: What|Removed |Added Priority|medium |high --- Comment #1 from Martin Peres --- This happened again: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4782/fi-kbl-8809g/igt@amdgpu/amd_cs_...@sync-fork-gfx0.html [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled seq=137, emitted seq=137 And it spilled(?) over the test igt@amdgpu_amd_prime@amd-to-i915: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4782/fi-kbl-8809g/igt@amdgpu_amd_pr...@amd-to-i915.html [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled seq=147, emitted seq=147 Since we did not get any feedback, I am bumping the priority a little in the hope to attract some attention. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/14] drm: clear plane damage during full modeset
On Thu, Sep 06, 2018 at 04:12:39PM +0200, Daniel Vetter wrote:
> On Thu, Sep 06, 2018 at 03:02:32PM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 05, 2018 at 04:38:50PM -0700, Deepak Rawat wrote:
> > > Plane damage is irrelevant when full modeset happens so clear the damage
> > > blob property(If set by user-space). With damage blob cleared damage
> > > helper iterator will return full plane src as damage clip.
> > >
> > > Signed-off-by: Deepak Rawat
> > > ---
> > > drivers/gpu/drm/drm_atomic_helper.c | 4
> > > include/drm/drm_damage_helper.h | 10 ++
> > > 2 files changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index be83e2763c18..e06d2d5d582f 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -31,6 +31,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > >
> > > #include "drm_crtc_helper_internal.h"
> > > @@ -88,6 +89,9 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> > > *state,
> > > return;
> > >
> > > crtc_state->planes_changed = true;
> > > +
> > > + if (drm_atomic_crtc_needs_modeset(crtc_state))
> > > + drm_plane_clear_damage(plane_state);
> >
> > So if we sometimes magically clear it, where do we reinitialize it with
> > the user provided damage for the next update?
> >
> > > }
> > > }
> > >
> > > diff --git a/include/drm/drm_damage_helper.h
> > > b/include/drm/drm_damage_helper.h
> > > index f1282b459a4f..1f988f7fdd72 100644
> > > --- a/include/drm/drm_damage_helper.h
> > > +++ b/include/drm/drm_damage_helper.h
> > > @@ -71,6 +71,16 @@ drm_plane_get_damage_clips(const struct
> > > drm_plane_state *state)
> > > state->fb_damage_clips->data : NULL);
> > > }
> > >
> > > +/**
> > > + * drm_plane_clear_damage - clears damage blob in a plane state
> > > + * @state: Plane state
> > > + */
> > > +static inline void drm_plane_clear_damage(struct drm_plane_state *state)
> > > +{
> > > + drm_property_blob_put(state->fb_damage_clips);
> > > + state->fb_damage_clips = NULL;
> >
> > Ah. So you're trying to clear out the user provided damage here. That
> > doesn't really seem like sane sematics to me. Either the user provided
> > damage should stick always instead of just sometimes, or it should
> > always be cleared once the update is done (ie. could clear in
> > duplicate_state() in that case).
>
> Read patch 2 more careful, we always reset damage when duplicating state.
> So it's one-shot (like fences).
Ah, thanks for pointing it out. Was actually in patch 1 :)
>
> But you might also want to ignore damage (and do a full upload) in other
> cases, like a modeset, or when you move your plane between crtc. This
> function here is for that.
>
> And yes this should be explained in the kerneldoc. And _clear_damage is
> not a great name for what it's meant to do, that's just what it does. I
> think in the past I've suggested _full_damage() or something like that.
> -Daniel
>
> >
> > > +}
> > > +
> > > void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
> > > int
> > > drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter
> > > *iter,
> > > --
> > > 2.17.1
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Ville Syrjälä
> > Intel
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107784] [AMD tahiti XT] displayport broken
https://bugs.freedesktop.org/show_bug.cgi?id=107784 --- Comment #16 from Michel Dänzer --- (In reply to Sylvain BERTRAND from comment #15) > Not consistent? Could you be more specific? You wrote: (In reply to Sylvain BERTRAND from comment #6) >- this commit is failing. >- the commit right before this one was working. "this commit" being 019cddc88f9e4ae0de2c76802f7137210c2101aa (the I2C merge), which has two parents. Both of those parent commits contain commit e2a9ca29b5edc89da2fddeae30e1070b272395c5 (a TSC commit) as part of their history. So you previously considered commit e2a9ca29b5edc89da2fddeae30e1070b272395c5 as both bad and good. That's the inconsistency. This most likely means that you're not yet able to reliably determine that a given commit is bad, e.g. due to not testing (long) enough. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/14] drm: clear plane damage during full modeset
On Thu, Sep 06, 2018 at 03:02:32PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 05, 2018 at 04:38:50PM -0700, Deepak Rawat wrote:
> > Plane damage is irrelevant when full modeset happens so clear the damage
> > blob property(If set by user-space). With damage blob cleared damage
> > helper iterator will return full plane src as damage clip.
> >
> > Signed-off-by: Deepak Rawat
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 4
> > include/drm/drm_damage_helper.h | 10 ++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index be83e2763c18..e06d2d5d582f 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -31,6 +31,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include "drm_crtc_helper_internal.h"
> > @@ -88,6 +89,9 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> > *state,
> > return;
> >
> > crtc_state->planes_changed = true;
> > +
> > + if (drm_atomic_crtc_needs_modeset(crtc_state))
> > + drm_plane_clear_damage(plane_state);
>
> So if we sometimes magically clear it, where do we reinitialize it with
> the user provided damage for the next update?
>
> > }
> > }
> >
> > diff --git a/include/drm/drm_damage_helper.h
> > b/include/drm/drm_damage_helper.h
> > index f1282b459a4f..1f988f7fdd72 100644
> > --- a/include/drm/drm_damage_helper.h
> > +++ b/include/drm/drm_damage_helper.h
> > @@ -71,6 +71,16 @@ drm_plane_get_damage_clips(const struct drm_plane_state
> > *state)
> >state->fb_damage_clips->data : NULL);
> > }
> >
> > +/**
> > + * drm_plane_clear_damage - clears damage blob in a plane state
> > + * @state: Plane state
> > + */
> > +static inline void drm_plane_clear_damage(struct drm_plane_state *state)
> > +{
> > + drm_property_blob_put(state->fb_damage_clips);
> > + state->fb_damage_clips = NULL;
>
> Ah. So you're trying to clear out the user provided damage here. That
> doesn't really seem like sane sematics to me. Either the user provided
> damage should stick always instead of just sometimes, or it should
> always be cleared once the update is done (ie. could clear in
> duplicate_state() in that case).
Read patch 2 more careful, we always reset damage when duplicating state.
So it's one-shot (like fences).
But you might also want to ignore damage (and do a full upload) in other
cases, like a modeset, or when you move your plane between crtc. This
function here is for that.
And yes this should be explained in the kerneldoc. And _clear_damage is
not a great name for what it's meant to do, that's just what it does. I
think in the past I've suggested _full_damage() or something like that.
-Daniel
>
> > +}
> > +
> > void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
> > int
> > drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter
> > *iter,
> > --
> > 2.17.1
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] intel: use drm namespace for exported functions
And add them to the list of exported function to fix the tests.
Fixes: 4e81d4f9c9b7fd6510cf "intel: add generic functions to check PCI ID"
Cc: Lucas De Marchi
Cc: Rodrigo Vivi
Signed-off-by: Eric Engestrom
---
Lucas, I'm assuming you meant to export those functions (and macros,
like `IS_GEN11()`), right?
If so, you need to namespace them, and add them to the list of allowed
exports.
---
intel/intel-symbol-check | 2 ++
intel/intel_bufmgr_gem.c | 2 +-
intel/intel_chipset.c| 4 ++--
intel/intel_chipset.h| 12 ++--
intel/intel_decode.c | 2 +-
5 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/intel/intel-symbol-check b/intel/intel-symbol-check
index 4d30a4b15feb94e9523d..339240a1bce7f9673cd3 100755
--- a/intel/intel-symbol-check
+++ b/intel/intel-symbol-check
@@ -87,11 +87,13 @@ drm_intel_gem_context_destroy
drm_intel_gem_context_get_id
drm_intel_get_aperture_sizes
drm_intel_get_eu_total
+drm_intel_get_genx
drm_intel_get_min_eu_in_pool
drm_intel_get_pipe_from_crtc_id
drm_intel_get_pooled_eu
drm_intel_get_reset_stats
drm_intel_get_subslice_total
+drm_intel_is_genx
drm_intel_reg_read
EOF
done)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index d6587b76ac004aa4a5a5..3190cb249acda0ae19ab 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -3656,7 +3656,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->gen = 7;
else if (IS_GEN8(bufmgr_gem->pci_device))
bufmgr_gem->gen = 8;
- else if (!intel_get_genx(bufmgr_gem->pci_device, _gem->gen)) {
+ else if (!drm_intel_get_genx(bufmgr_gem->pci_device, _gem->gen))
{
free(bufmgr_gem);
bufmgr_gem = NULL;
goto exit;
diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
index d5c33cc59002e1f150e8..89f69ed9e6ed67d8c3fc 100644
--- a/intel/intel_chipset.c
+++ b/intel/intel_chipset.c
@@ -44,7 +44,7 @@ static const struct pci_device {
INTEL_SKL_IDS(9),
};
-bool intel_is_genx(unsigned int devid, int gen)
+bool drm_intel_is_genx(unsigned int devid, int gen)
{
const struct pci_device *p,
*pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
@@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
return false;
}
-bool intel_get_genx(unsigned int devid, int *gen)
+bool drm_intel_get_genx(unsigned int devid, int *gen)
{
const struct pci_device *p,
*pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
index 9b1e64f1f4bd19b3cf38..01488be2fb34fd08a876 100644
--- a/intel/intel_chipset.h
+++ b/intel/intel_chipset.h
@@ -330,12 +330,12 @@
/* New platforms use kernel pci ids */
#include
-bool intel_is_genx(unsigned int devid, int gen);
-bool intel_get_genx(unsigned int devid, int *gen);
+bool drm_intel_is_genx(unsigned int devid, int gen);
+bool drm_intel_get_genx(unsigned int devid, int *gen);
-#define IS_GEN9(devid) intel_is_genx(devid, 9)
-#define IS_GEN10(devid) intel_is_genx(devid, 10)
-#define IS_GEN11(devid) intel_is_genx(devid, 11)
+#define IS_GEN9(devid) drm_intel_is_genx(devid, 9)
+#define IS_GEN10(devid) drm_intel_is_genx(devid, 10)
+#define IS_GEN11(devid) drm_intel_is_genx(devid, 11)
#define IS_9XX(dev)(IS_GEN3(dev) || \
IS_GEN4(dev) || \
@@ -343,6 +343,6 @@ bool intel_get_genx(unsigned int devid, int *gen);
IS_GEN6(dev) || \
IS_GEN7(dev) || \
IS_GEN8(dev) || \
-intel_get_genx(dev, NULL))
+drm_intel_get_genx(dev, NULL))
#endif /* _INTEL_CHIPSET_H */
diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 0ff095bc6285d03f4fed..4a493cbfa8f84591b994 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -3823,7 +3823,7 @@ drm_intel_decode_context_alloc(uint32_t devid)
ctx->devid = devid;
ctx->out = stdout;
- if (intel_get_genx(devid, >gen))
+ if (drm_intel_get_genx(devid, >gen))
;
else if (IS_GEN8(devid))
ctx->gen = 8;
--
Cheers,
Eric
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] CI: Capture test logs as GitLab artifacts
On 6 September 2018 at 12:02, Eric Engestrom wrote: > On Thursday, 2018-09-06 11:01:17 +0100, Daniel Stone wrote: >> GitLab CI already captures all the stdout/stderr output from the build >> process as the log. However, some other important information is hidden >> in other log files. >> >> Taken from Wayland, capture logs from the configuration process as well >> as from every check. >> >> Signed-off-by: Daniel Stone >> Cc: Rodrigo Vivi >> Cc: Lucas De Marchi >> Cc: Eric Engeström >> Cc: Daniel Vetter >> --- >> .gitlab-ci.yml | 26 ++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml >> index eee6abfc..50ec8527 100644 >> --- a/.gitlab-ci.yml >> +++ b/.gitlab-ci.yml >> @@ -1,6 +1,29 @@ >> +.artifacts-meson: > > Ooooh, variables? I might try to dedup the 'meson build' and > 'autotools build' code with those :) > >> + when: always >> + paths: >> + - _build/meson-logs >> + >> +.artifacts-autotools: >> + when: always >> + paths: >> +- _build/*.log >> +- _build/amdgpu/*.log >> +- _build/etnaviv/*.log >> +- _build/exynos/*.log >> +- _build/freedreno/*.log >> +- _build/intel/*.log >> +- _build/libkms/*.log >> +- _build/nouveau/*.log >> +- _build/omap/*.log >> +- _build/radeon/*.log >> +- _build/tegra/*.log > > All of the above can be simplified to: > - _build/*/*.log > > (vc4 is missing btw) > > With vc4 added, or /*/ used: > Acked-by: Eric Engestrom > I would fold everything in these tree lines. - _build/*.log - _build/*/*.log - _build/*/*/*.log It covers everything existing, plus catches future ones ;-) With that Reviewed-by: Emil Velikov -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] gitlab-ci: use variables to deduplicate the build commands
Signed-off-by: Eric Engestrom --- .gitlab-ci.yml | 129 ++--- 1 file changed, 47 insertions(+), 82 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index eee6abfcdd7de2839660..1dc434a5d359b3b077e7 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,3 +1,46 @@ +.meson-build: + - meson _build + -D amdgpu=true + -D cairo-tests=true + -D etnaviv=true + -D exynos=true + -D freedreno=true + -D freedreno-kgsl=true + -D intel=true + -D libkms=true + -D man-pages=true + -D nouveau=true + -D omap=true + -D radeon=true + -D tegra=true + -D udev=true + -D valgrind=true + -D vc4=true + -D vmwgfx=true + - ninja -C _build + - ninja -C _build test + +.autotools-build: + - mkdir _build + - cd _build + - ../autogen.sh + --enable-udev + --enable-libkms + --enable-intel + --enable-radeon + --enable-admgpu + --enable-nouveau + --enable-vmwfgx + --enable-omap-experimental-api + --enable-exynos-experimental-api + --enable-freedreno + --enable-freedreno-kgsl + --enable-tegra-experimental-api + --enable-vc4 + --enable-etnaviv-experimental-api + - make + - make check + latest-meson: stage: build image: base/archlinux:latest @@ -10,27 +53,7 @@ latest-meson: valgrind libatomic_ops cairo cunit - script: -- meson _build --D amdgpu=true --D cairo-tests=true --D etnaviv=true --D exynos=true --D freedreno=true --D freedreno-kgsl=true --D intel=true --D libkms=true --D man-pages=true --D nouveau=true --D omap=true --D radeon=true --D tegra=true --D udev=true --D valgrind=true --D vc4=true --D vmwgfx=true -- ninja -C _build -- ninja -C _build test + script: *meson-build latest-autotools: stage: build @@ -45,26 +68,7 @@ latest-autotools: cairo cunit xorg-util-macros git # autogen.sh depends on git - script: -- mkdir _build -- cd _build -- ../autogen.sh ---enable-udev ---enable-libkms ---enable-intel ---enable-radeon ---enable-admgpu ---enable-nouveau ---enable-vmwfgx ---enable-omap-experimental-api ---enable-exynos-experimental-api ---enable-freedreno ---enable-freedreno-kgsl ---enable-tegra-experimental-api ---enable-vc4 ---enable-etnaviv-experimental-api -- make -- make check + script: *autotools-build oldest-meson: stage: build @@ -98,29 +102,9 @@ oldest-meson: (cd $LIBPCIACCESS_VERSION && ./configure --prefix=$HOME/prefix && make install) - pip3 install wheel setuptools - pip3 install meson==0.43 - script: - export PKG_CONFIG_PATH=$HOME/prefix/lib/pkgconfig:$HOME/prefix/share/pkgconfig - export LD_LIBRARY_PATH="$HOME/prefix/lib:$LD_LIBRARY_PATH" -- meson _build --D amdgpu=true --D cairo-tests=true --D etnaviv=true --D exynos=true --D freedreno=true --D freedreno-kgsl=true --D intel=true --D libkms=true --D man-pages=true --D nouveau=true --D omap=true --D radeon=true --D tegra=true --D udev=true --D valgrind=true --D vc4=true --D vmwgfx=true -- ninja -C _build -- ninja -C _build test + script: *meson-build oldest-autotools: stage: build @@ -155,25 +139,6 @@ oldest-autotools: wget --no-check-certificate https://xorg.freedesktop.org/releases/individual/lib/$LIBPCIACCESS_VERSION.tar.bz2 && tar -jxvf $LIBPCIACCESS_VERSION.tar.bz2 && (cd $LIBPCIACCESS_VERSION && ./configure --prefix=$HOME/prefix && make install) - script: - export PKG_CONFIG_PATH=$HOME/prefix/lib/pkgconfig:$HOME/prefix/share/pkgconfig - export LD_LIBRARY_PATH="$HOME/prefix/lib:$LD_LIBRARY_PATH" -- mkdir _build -- cd _build -- ../autogen.sh ---enable-udev ---enable-libkms ---enable-intel ---enable-radeon ---enable-admgpu ---enable-nouveau ---enable-vmwfgx ---enable-omap-experimental-api ---enable-exynos-experimental-api ---enable-freedreno ---enable-freedreno-kgsl ---enable-tegra-experimental-api ---enable-vc4 ---enable-etnaviv-experimental-api -- make -- make check + script: *autotools-build -- Cheers, Eric ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Update todo.rst
On 6 September 2018 at 10:40, Heiko Stuebner wrote: > Am Mittwoch, 5. September 2018, 20:15:09 CEST schrieb Daniel Vetter: >> - drmP.h is now fully split up. >> - vkms is happening (and will gain its own todo and docs under a new >> vkms.rst file real soon) >> - legacy cruft is completely hidden now, drm_vblank.c is split out >> from drm_irq.c now. I've decided to drop the task to split out >> drm_legacy.ko, partially because Dave already rejected a patch to >> hide the old dri1 drivers better. Current state feels good enough to >> me. >> - best_encoder atomic cleanup is done (it's now the default, not even >> exported anymore) >> - bunch of smaller things >> >> v2: >> - Explain why the drm_legacy.ko task is dropped (Emil). >> - typos (Sam). >> >> v3: Fix typo (Ilia) >> >> Cc: Ilia Mirkin >> Cc: Sam Ravnborg >> Cc: Emil Velikov >> Signed-off-by: Daniel Vetter >> Cc: Gustavo Padovan >> Cc: Maarten Lankhorst >> Cc: Sean Paul >> Cc: David Airlie > > I've read through the text changes and didn't spot any glaring typos > [beware non-native speaker], so fwiw Most of the people in the CC list are ;-) Fwiw Reviewed-by: Emil Velikov Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.
https://bugs.freedesktop.org/show_bug.cgi?id=105733 --- Comment #37 from markusr...@gmail.com --- (In reply to markusraat from comment #36) > (In reply to markusraat from comment #35) > > It might be that kernel option apci=ht ( also apci=off ) solve the problem. > > It is taking more time to waiting the possible problem appearance. At least > > it worth of testing. But this is not maybe the final solution for this bug? > > > > [0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-4.18.5-041805-generic > > root=UUID=c3df607f-ac6e-11e8-9f6b-3497f638e103 ro acpi=ht > > [0.00] Malformed early option 'acpi' > > Okay, the "acpi=off" or "acpi=ht" was the miss shot. > > But changing from motherboard bios GPU PCIe speed auto > gen3 is giving very > promissing results! I also rose logging level from grub settings to > "loglevel=8" but I haven't got regenerated the crash. I will reply if this > fails again. Nope, Sep 6 16:04:31 x99 org.gnome.Shell.desktop[2332]: [Child 18594, MediaPlayback #2] WARNING: Decoder=7fbcd7976d40 Decode error: NS_ERROR_DOM_MEDIA_FATAL_ERR (0x806e0005) - RefPtr, mozilla::MediaResult, true> > mozilla::MediaSourceTrackDemuxer::DoGetSamples(int32_t): manager is detached.: file /build/firefox-oscv9o/firefox-61.0.1+build1/dom/media/MediaDecoderStateMachine.cpp, line 3411 Sep 6 16:04:31 x99 org.gnome.Shell.desktop[2332]: [Child 18594, MediaPlayback #1] WARNING: Decoder=7fbcd7976d40 Decode error: NS_ERROR_DOM_MEDIA_FATAL_ERR (0x806e0005) - RefPtr, mozilla::MediaResult, true> > mozilla::MediaSourceTrackDemuxer::DoGetSamples(int32_t): manager is detached.: file /build/firefox-oscv9o/firefox-61.0.1+build1/dom/media/MediaDecoderStateMachine.cpp, line 3411 Sep 6 16:04:31 x99 org.gnome.Shell.desktop[2332]: [Child 18594, MediaPlayback #3] WARNING: Decoder=7fbcd7976d40 Decode error: NS_ERROR_DOM_MEDIA_FATAL_ERR (0x806e0005) - RefPtr, mozilla::MediaResult, true> > mozilla::MediaSourceTrackDemuxer::DoGetSamples(int32_t): manager is detached.: file /build/firefox-oscv9o/firefox-61.0.1+build1/dom/media/MediaDecoderStateMachine.cpp, line 3411 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 107784] [AMD tahiti XT] displayport broken
On Thu, Sep 06, 2018 at 10:04:53AM +, bugzilla-dae...@freedesktop.org wrote: > https://bugs.freedesktop.org/show_bug.cgi?id=107784 > > --- Comment #14 from Michel Dänzer --- > That's not consistent with the merge commit you identified earlier. So I'm > afraid it's likely that you incorrectly classified some commits as good or > bad. > Maybe the problem doesn't occur 100% consistently even with bad commits, so > try > testing longer / more times before declaring a commit as good. Not consistent? Could you be more specific? Some git magic I forgot again? This time I used git bisect to go through "merge" commits properly. I did test countless times those commits: that would mean this TSC code would "side-effect" an ultra-rare bad condition into an nearly-all-the-time bad condition. That amount of bad luck? Whatever, I'll update amd-staging-drm-next, and go through a full bisection again. I'll need probably days (lucky: I don't work). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107784] [AMD tahiti XT] displayport broken
https://bugs.freedesktop.org/show_bug.cgi?id=107784 --- Comment #15 from Sylvain BERTRAND --- On Thu, Sep 06, 2018 at 10:04:53AM +, bugzilla-dae...@freedesktop.org wrote: > https://bugs.freedesktop.org/show_bug.cgi?id=107784 > > --- Comment #14 from Michel Dänzer --- > That's not consistent with the merge commit you identified earlier. So I'm > afraid it's likely that you incorrectly classified some commits as good or > bad. > Maybe the problem doesn't occur 100% consistently even with bad commits, so > try > testing longer / more times before declaring a commit as good. Not consistent? Could you be more specific? Some git magic I forgot again? This time I used git bisect to go through "merge" commits properly. I did test countless times those commits: that would mean this TSC code would "side-effect" an ultra-rare bad condition into an nearly-all-the-time bad condition. That amount of bad luck? Whatever, I'll update amd-staging-drm-next, and go through a full bisection again. I'll need probably days (lucky: I don't work). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] of/platform: initialise AMBA default DMA masks
On 31/08/18 15:13, Linus Walleij wrote:
This addresses a v4.19-rc1 regression in the PL111 DRM driver
in drivers/gpu/pl111/*
The driver uses the CMA KMS helpers and will thus at some
point call down to dma_alloc_attrs() to allocate a chunk
of contigous DMA memory for the framebuffer.
It appears that in v4.18, it was OK that this (and other
DMA mastering AMBA devices) left dev->coherent_dma_mask
blank (zero).
In v4.19-rc1 the WARN_ON_ONCE(dev && !dev->coherent_dma_mask)
in dma_alloc_attrs() in include/linux/dma-mapping.h is
triggered. The allocation later fails when get_coherent_dma_mask()
is called from __dma_alloc() and __dma_alloc() returns
NULL:
drm-clcd-pl111 dev:20: coherent DMA mask is unset
drm-clcd-pl111 dev:20: [drm:drm_fb_helper_fbdev_setup] *ERROR*
Failed to set fbdev configuration
It turns out that in commit 4d8bde883bfb
("OF: Don't set default coherent DMA mask")
the OF core stops setting the default DMA mask on new devices,
especially those lines of the patch:
- if (!dev->coherent_dma_mask)
- dev->coherent_dma_mask = DMA_BIT_MASK(32);
Robin Murphy solved a similar problem in
a5516219b102 ("of/platform: Initialise default DMA masks")
by simply assigning dev.coherent_dma_mask and the
dev.dma_mask to point to the same when creating devices
from the device tree, and introducing the same code into
the code path creating AMBA/PrimeCell devices solved my
problem, graphics now come up.
Ugh, sorry - that commit really should have updated
of_amba_device_create() at the same time, but thanks to the tangled
history I managed to overlook it. And of course, the one PrimeCell
device in my usual test system (PL330) gets an explicit coherent mask
set by its driver so I didn't get the WARN() to remind me...
I see this is merged already, but after-the-fact Ack anyway. Apologies
for the breakage, and thanks for fixing my mess :)
Robin.
The code simply assumes that the device can access all
of the system memory by setting the coherent DMA mask
to 0x when creating a device from the device
tree, which is crude, but seems to be what kernel v4.18
assumed.
The AMBA PrimeCells do not differ between coherent and
streaming DMA so we can just assign the same to any
DMA mask.
Possibly drivers should augment their coherent DMA mask
in accordance with "dma-ranges" from the device tree
if more finegranular masking is needed.
Reported-by: Russell King
Fixes: 4d8bde883bfb ("OF: Don't set default coherent DMA mask")
Cc: Russell King
Cc: Christoph Hellwig
Cc: Robin Murphy
Signed-off-by: Linus Walleij
---
ChangeLog v2->v3:
- Provide proper root cause analysis, point to the right
offending commit with Fixes:
- Make even more elaborate description of what is causing
this.
ChangeLog v1->v2:
- Provide a better description for the change.
---
drivers/of/platform.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7ba90c290a42..6c59673933e9 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -241,6 +241,10 @@ static struct amba_device *of_amba_device_create(struct
device_node *node,
if (!dev)
goto err_clear_flag;
+ /* AMBA devices only support a single DMA mask */
+ dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+ dev->dev.dma_mask = >dev.coherent_dma_mask;
+
/* setup generic device info */
dev->dev.of_node = of_node_get(node);
dev->dev.fwnode = >fwnode;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] drm/bridge: use bus flags in bridge timings
Hi Linus, On Thursday, 6 September 2018 14:07:41 EEST Linus Walleij wrote: > On Wed, Sep 5, 2018 at 8:32 PM Stefan Agner wrote: > > On 05.09.2018 00:44, Laurent Pinchart wrote: > > > > Good point! I actually really don't like that we use the same flags here > > but from a different perspective. Especially since the flags defines > > document things differently: > > > > /* drive data on pos. edge */ > > #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) > > /* drive data on neg. edge */ > > #define DRM_BUS_FLAG_PIXDATA_NEGEDGE(1<<3) > > Maybe a stupid comment from my side, but can't we just change the > documentation to match the usecases? > > /* Trigger pixel data latch on positive edge */ > #define DRM_BUS_FLAG_PIXDATA_POSEDGE(1<<2) The flags are used for the drm_connector bus_flags field, and really mean driving on the positive/negative edges. We this can't change their meaning like this. > > Using the opposite perspective would also need translation in crtc > > drivers... So far no driver uses sampling_edge. > > > > I would prefer if we always use the meaning as documented by the flags. > > > > I guess we would need to convert DRM_BUS_FLAG_PIXDATA_POSEDGE -> > > DRM_BUS_FLAG_PIXDATA_NEGEDGE. > > > > Linus Walleij, you added sampling edge, any thoughts? > > I just thought it was generally useful to have triggering edge encoded > into the bridge as it makes it clear that this edge is something > that is a delayed version of the driving edge which is subject to > clock skew caused by the speed of electrons in silicon and > copper and slew rate caused by parasitic capacitance. I agree that we need both the driving and sampling edge. In many case they will be opposite, and providing some kind of appropriate defaults in APIs is fine by me, but we need a way to specify both when needed. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Print erroneous hscale/vscale on failure
On Thu, Sep 06, 2018 at 02:30:24PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 05, 2018 at 04:46:01PM -0400, Sean Paul wrote:
> > From: Sean Paul
> >
> > I ran across this last week when I was trying to get this function to
> > work. It's useful to have the scale values in the log upon failure.
> >
> > Signed-off-by: Sean Paul
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 2c23a48482da..1725546d5105 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -758,7 +758,8 @@ int drm_atomic_helper_check_plane_state(struct
> > drm_plane_state *plane_state,
> > hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> > vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> > if (hscale < 0 || vscale < 0) {
> > - DRM_DEBUG_KMS("Invalid scaling of plane\n");
> > + DRM_DEBUG_KMS("Invalid scaling of plane (%d/%d)\n",
> > + hscale, vscale);
>
> I think it's just going to be -ERANGE for at least one of them.
> So not quite sure what extra benefit we get from this. What
> might be more helpful is printing the actual computed scale
> factor and the min/max.
Good point, it will tell you which scale is invalid, but not necessarily why.
I'll tweak the args for drm_rect_calc_*scale to return -errno and pass *scale by
pointer.
v2 incoming
Sean
>
> > drm_rect_debug_print("src: ", _state->src, true);
> > drm_rect_debug_print("dst: ", _state->dst, false);
> > return -ERANGE;
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #30 from L.Y. Sim --- I have this issue on a 3840x1600 Acer XR382CQK with an RX560 with Kernel 4.18.5-1 on Manjaro. When I set the refresh rate to 75Hz, severe artifacts and flickering appear. Both echo high > /sys/class/drm/card0/device/power_dpm_force_performance_level and echo low > /sys/class/drm/card0/device/power_dpm_force_performance_level stop the flickering and artifacting, and I can see via cat /sys/class/drm/card0/device/pp_dpm_mclk that the memory clocks are set to 1750Mhz and 300Mhz respectively. However, if /sys/class/drm/card0/device/power_dpm_force_performance_level is set to auto, I can see (via watching /sys/class/drm/card0/device/pp_dpm_mclk with time intervals around 0.1s), that the memory clock oscillates rapidly between 300Mhz, 625Mhz and 1750Mhz. So it seems to me that the rapid change in memory frequency is what's causing the flickering. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/14] drm: clear plane damage during full modeset
On Wed, Sep 05, 2018 at 04:38:50PM -0700, Deepak Rawat wrote:
> Plane damage is irrelevant when full modeset happens so clear the damage
> blob property(If set by user-space). With damage blob cleared damage
> helper iterator will return full plane src as damage clip.
>
> Signed-off-by: Deepak Rawat
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 4
> include/drm/drm_damage_helper.h | 10 ++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index be83e2763c18..e06d2d5d582f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "drm_crtc_helper_internal.h"
> @@ -88,6 +89,9 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state,
> return;
>
> crtc_state->planes_changed = true;
> +
> + if (drm_atomic_crtc_needs_modeset(crtc_state))
> + drm_plane_clear_damage(plane_state);
So if we sometimes magically clear it, where do we reinitialize it with
the user provided damage for the next update?
> }
> }
>
> diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
> index f1282b459a4f..1f988f7fdd72 100644
> --- a/include/drm/drm_damage_helper.h
> +++ b/include/drm/drm_damage_helper.h
> @@ -71,6 +71,16 @@ drm_plane_get_damage_clips(const struct drm_plane_state
> *state)
> state->fb_damage_clips->data : NULL);
> }
>
> +/**
> + * drm_plane_clear_damage - clears damage blob in a plane state
> + * @state: Plane state
> + */
> +static inline void drm_plane_clear_damage(struct drm_plane_state *state)
> +{
> + drm_property_blob_put(state->fb_damage_clips);
> + state->fb_damage_clips = NULL;
Ah. So you're trying to clear out the user provided damage here. That
doesn't really seem like sane sematics to me. Either the user provided
damage should stick always instead of just sometimes, or it should
always be cleared once the update is done (ie. could clear in
duplicate_state() in that case).
> +}
> +
> void drm_plane_enable_fb_damage_clips(struct drm_plane *plane);
> int
> drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter
> *iter,
> --
> 2.17.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
