Re: [linux-sunxi] Re: [PATCH 13/27] drm/sun4i: Add support for H6 DE3 mixer 0
Dne sobota, 22. september 2018 ob 15:47:03 CEST je Chen-Yu Tsai napisal(a): > On Sat, Sep 22, 2018 at 9:23 PM Chen-Yu Tsai wrote: > > On Sun, Sep 2, 2018 at 3:27 PM Jernej Skrabec wrote: > > > Mixer 0 has 1 VI and 3 UI planes, scaler on all planes and can output > > > 4K image @60Hz. It also support 10 bit colors. > > > > AFAICT 10 bit color support is not implemented? Please mention this. ok. > > > > > Signed-off-by: Jernej Skrabec > > > --- > > > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 + > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index a9218abf0935..54eca2dd4b33 > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > @@ -540,6 +540,15 @@ static int sun8i_mixer_remove(struct > > > platform_device *pdev)> > > > > return 0; > > > > > > } > > > > > > +static const struct sun8i_mixer_cfg sun50i_h6_mixer0_cfg = { > > > > Please sort the per-compatible structures according to "version sort" > > rules.> > > > + .ccsc = 0, > > > + .is_de3 = true, > > > + .mod_rate = 6, > > > + .scaler_mask= 0xf, > > > + .ui_num = 3, > > > + .vi_num = 1, > > > +}; > > > + > > > > > > static const struct sun8i_mixer_cfg sun8i_a83t_mixer0_cfg = { > > > > > > .ccsc = 0, > > > .scaler_mask= 0xf, > > > > > > @@ -587,6 +596,10 @@ static const struct sun8i_mixer_cfg > > > sun8i_v3s_mixer_cfg = {> > > > > }; > > > > > > static const struct of_device_id sun8i_mixer_of_table[] = { > > > > > > + { > > > + .compatible = "allwinner,sun50i-h6-de3-mixer-0", > > > + .data = &sun50i_h6_mixer0_cfg, > > > + }, > > > > Same here. > > > > ChenYu > > BTW, DE 3.0 includes a register in DE TOP called "DE IP configure register", > which gives the number of IP blocks per class, per mixer. If we retrieve > the configuration from this register, then we shouldn't need to > differentiate between mixer-0 and mixer-1 with compatible strings. > > What do you think? IIRC, not all setting were correct when read from registers, but I would need to check again. I'm also not sure if register holds all possible settings, so it is safer to have separate list. We would also have to devise mechanism to get this data from DE2/3 CCU driver (it occupies the same memory space). Perhaps the strongest argument is that some SoCs with DE3 have HW bug in mixer1 block, including that in H6. In order to work, mod clock has to be enabled for mixer0 and mixer1 at the same time. I would associate that quirk with mixer1 compatible. Best regards, Jernej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/27] drm/sun4i: Add basic support for DE3
Dne sobota, 22. september 2018 ob 15:19:12 CEST je Chen-Yu Tsai napisal(a): > On Sun, Sep 2, 2018 at 3:27 PM Jernej Skrabec wrote: > > Display Engine 3 is an upgrade of DE2 with new features like support for > > 10 bit color formats and support for AFBC. > > > > Most of DE2 code works with DE3, except some small details. > > > > Add support for it. > > > > Signed-off-by: Jernej Skrabec > > --- > > > > drivers/gpu/drm/sun4i/sun8i_csc.c | 96 +++-- > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 17 - > > drivers/gpu/drm/sun4i/sun8i_mixer.h | 21 +- > > drivers/gpu/drm/sun4i/sun8i_ui_scaler.c | 8 ++- > > drivers/gpu/drm/sun4i/sun8i_ui_scaler.h | 1 + > > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 8 +++ > > drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 2 + > > drivers/gpu/drm/sun4i/sun8i_vi_scaler.c | 34 - > > drivers/gpu/drm/sun4i/sun8i_vi_scaler.h | 23 ++ > > 9 files changed, 197 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_csc.c > > b/drivers/gpu/drm/sun4i/sun8i_csc.c index b14925b40ccf..101901ccf2dc > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_csc.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_csc.c > > @@ -34,6 +34,34 @@ static const u32 yvu2rgb[] = { > > > > 0x04A8, 0x, 0x0813, 0xFFFBAC4A, > > > > }; > > > > +/* > > + * DE3 has a bit different CSC units. Factors are in two's complement > > format. + * First three have 17 bits for fractinal part and last two 2 > > bits. First + * three values in each line are multiplication factor, 4th > > is difference, + * which is subtracted from the input value before the > > multiplication and + * last value is constant, which is added at the end. > > + * > > + * x' = c00 * (x + d0) + c01 * (y + d1) + c02 * (z + d2) + const0 > > + * y' = c10 * (x + d0) + c11 * (y + d1) + c12 * (z + d2) + const1 > > + * z' = c20 * (x + d0) + c21 * (y + d1) + c22 * (z + d2) + const2 > > + * > > + * Please note that above formula is true only for Blender CSC. Other DE3 > > CSC + * units takes only positive value for difference. From what can be > > deducted + * from BSP driver code, those units probably automatically > > assume that + * difference has to be subtracted. > > + */ > > +static const u32 yuv2rgb_de3[] = { > > + 0x0002542a, 0x, 0x0003312a, 0xffc0, 0x, > > + 0x0002542a, 0x376b, 0xfffe5fc3, 0xfe00, 0x, > > + 0x0002542a, 0x000408d3, 0x, 0xfe00, 0x, > > +}; > > + > > +static const u32 yvu2rgb_de3[] = { > > + 0x0002542a, 0x0003312a, 0x, 0xffc0, 0x, > > + 0x0002542a, 0xfffe5fc3, 0x376b, 0xfe00, 0x, > > + 0x0002542a, 0x, 0x000408d3, 0xfe00, 0x, > > +}; > > + > > > > static void sun8i_csc_set_coefficients(struct regmap *map, u32 base, > > > >enum sun8i_csc_mode mode) > > > > { > > > > @@ -61,6 +89,38 @@ static void sun8i_csc_set_coefficients(struct regmap > > *map, u32 base,> > > } > > > > } > > > > +static void sun8i_de3_ccsc_set_coefficients(struct regmap *map, int > > layer, > > + enum sun8i_csc_mode mode) > > +{ > > + const u32 *table; > > + int i, j; > > + > > + switch (mode) { > > + case SUN8I_CSC_MODE_YUV2RGB: > > + table = yuv2rgb_de3; > > + break; > > + case SUN8I_CSC_MODE_YVU2RGB: > > + table = yvu2rgb_de3; > > + break; > > + default: > > + DRM_WARN("Wrong CSC mode specified.\n"); > > + return; > > + } > > + > > + for (i = 0; i < 3; i++) { > > + for (j = 0; j < 3; j++) > > + regmap_write(map, > > + > > SUN8I_MIXER_BLEND_CSC_COEFF(DE3_BLD_BASE, + > > layer, i, j), + > > table[i * 5 + j]); > > Given that the first three values occupy contiguous addresses, > you can use regmap_bulk_write() here. > > > + regmap_write(map, > > +SUN8I_MIXER_BLEND_CSC_CONST(DE3_BLD_BASE, > > +layer, i), > > +SUN8I_MIXER_BLEND_CSC_CONST_VAL(table[i * 5 + > > 3], +table[i > > * 5 + 4])); > Nit: Using a two-dimension array might make it easier to read. > > > + } > > +} > > + > > > > static void sun8i_csc_enable(struct regmap *map, u32 base, bool enable) > > { > > > > u32 val; > > > > @@ -73,21 +133,45 @@ static void sun8i_csc_enable(struct regmap *map, u32 > > base, bool enable)> > > regmap_update_bits(map, SUN8I_CSC_CTRL(base), SUN8I_CSC_CTRL_EN, > > val); > > > > } > > > > +static void sun8i_de3_ccsc_enable(str
Re: [PATCH 21/27] drm/sun4i: Add support for H6 HDMI PHY
Dne sobota, 22. september 2018 ob 17:55:35 CEST je Chen-Yu Tsai napisal(a): > On Sun, Sep 2, 2018 at 3:28 PM Jernej Skrabec wrote: > > H6 has Synopsys DWC HDMI 2.0 TX PHY. > > > > mpll settings were calculated from specifications of similar Synopsys > > HDMI PHY found in i.MX6. Other PHY settings were derived from BSP PHY > > driver code. > > > > Signed-off-by: Jernej Skrabec > > --- > > > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 137 + > > 1 file changed, 137 insertions(+) > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index ee2bf61cd4d2..2f5499bd35ec > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > @@ -14,6 +14,122 @@ > > > > */ > > > > #define I2C_ADDR 0x69 > > > > +static const struct dw_hdmi_mpll_config sun50i_h6_mpll_cfg[] = { > > How did you choose the pixel clock points for this table? The values > sort of match up with the BSP. I used this script: https://chromium.googlesource.com/chromiumos/platform/drm-tests/+/master/ rk3288_hdmitables.py I forgot already how I came to conclusion that there are identical register settings. After all, imx6 manual describes DWC HDMI PHY and it seems that settings are identical for DWC HDMI PHY contained in imx6, rk3288 and h6. Check out that script, it has some explanation how values are calculated. Part of the reason why I didn't use original settings is that it is pain to convert BSP PHY settings table to this form. Not all frequencies have settings for 8, 10 and 12 bit depth. > > > + { > > + 30666000, { > > + { 0x00b3, 0x }, > > + { 0x2153, 0x }, > > + { 0x40f3, 0x }, > > + }, > > + }, { > > + 3680, { > > + { 0x00b3, 0x }, > > + { 0x2153, 0x }, > > + { 0x40a2, 0x0001 }, > > + }, > > + }, { > > + 4600, { > > + { 0x00b3, 0x }, > > + { 0x2142, 0x0001 }, > > + { 0x40a2, 0x0001 }, > > + }, > > + }, { > > + 61333000, { > > + { 0x0072, 0x0001 }, > > + { 0x2142, 0x0001 }, > > + { 0x40a2, 0x0001 }, > > + }, > > + }, { > > + 7360, { > > + { 0x0072, 0x0001 }, > > + { 0x2142, 0x0001 }, > > + { 0x4061, 0x0002 }, > > + }, > > + }, { > > + 9200, { > > + { 0x0072, 0x0001 }, > > + { 0x2145, 0x0002 }, > > + { 0x4061, 0x0002 }, > > + }, > > + }, { > > + 122666000, { > > + { 0x0051, 0x0002 }, > > + { 0x2145, 0x0002 }, > > + { 0x4061, 0x0002 }, > > + }, > > + }, { > > + 14720, { > > + { 0x0051, 0x0002 }, > > + { 0x2145, 0x0002 }, > > + { 0x4064, 0x0003 }, > > + }, > > + }, { > > + 18400, { > > + { 0x0051, 0x0002 }, > > + { 0x214c, 0x0003 }, > > + { 0x4064, 0x0003 }, > > + }, > > + }, { > > + 22000, { > > + { 0x0040, 0x0003 }, > > + { 0x214c, 0x0003 }, > > + { 0x4064, 0x0003 }, > > + }, > > + }, { > > + 27200, { > > + { 0x0040, 0x0003 }, > > + { 0x214c, 0x0003 }, > > + { 0x5a64, 0x0003 }, > > + }, > > + }, { > > + 34000, { > > + { 0x0040, 0x0003 }, > > + { 0x3b4c, 0x0003 }, > > + { 0x5a64, 0x0003 }, > > + }, > > + }, { > > + 6, { > > 59400 is the proper clock rate. > > > + { 0x1a40, 0x0003 }, > > + { 0x3b4c, 0x0003 }, > > + { 0x5a64, 0x0003 }, > > + }, > > + }, { > > + ~0UL, { > > + { 0x, 0x }, > > + { 0x, 0x }, > > + { 0x, 0x }, > > + }, > > + } > > +}; > > + > > +static const struct dw_hdmi_curr_ctrl sun50i_h6_cur_ctr[] = { > > The BSP sometimes uses different settings depending on the pixel repetition. > Any ideas about this? I assume it's because the TMDS clock changes as a > result of pixel repetition, as is the s
Re: [linux-sunxi] [PATCH 11/27] drm/sun4i: Rework DE2 register defines
Dne sobota, 22. september 2018 ob 14:32:30 CEST je Chen-Yu Tsai napisal(a): > Hi, > > On Sun, Sep 2, 2018 at 3:27 PM Jernej Skrabec wrote: > > Most, if not all, registers found in DE2 still exists in DE3. However, > > units are on different base addresses. > > > > To prepare for addition of DE3 support, registers macros are reworked so > > they take base address as parameter. > > > > Signed-off-by: Jernej Skrabec > > [rebased] > > Signed-off-by: Icenowy Zheng > > This patch mostly checks out. But see below. > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h > > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 406c42e752d7..020b0a097c84 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > > @@ -29,20 +29,24 @@ > > > > #define SUN8I_MIXER_GLOBAL_DBUFF_ENABLEBIT(0) > > > > -#define SUN8I_MIXER_BLEND_PIPE_CTL 0x1000 > > -#define SUN8I_MIXER_BLEND_ATTR_FCOLOR(x) (0x1004 + 0x10 * (x) + > > 0x0) > > -#define SUN8I_MIXER_BLEND_ATTR_INSIZE(x) (0x1004 + 0x10 * (x) + > > 0x4) > > -#define SUN8I_MIXER_BLEND_ATTR_COORD(x)(0x1004 + 0x10 * > > (x) + 0x8) -#define SUN8I_MIXER_BLEND_ROUTE0x1080 > > -#define SUN8I_MIXER_BLEND_PREMULTIPLY 0x1084 > > -#define SUN8I_MIXER_BLEND_BKCOLOR 0x1088 > > -#define SUN8I_MIXER_BLEND_OUTSIZE 0x108c > > -#define SUN8I_MIXER_BLEND_MODE(x) (0x1090 + 0x04 * (x)) > > -#define SUN8I_MIXER_BLEND_CK_CTL 0x10b0 > > -#define SUN8I_MIXER_BLEND_CK_CFG 0x10b4 > > -#define SUN8I_MIXER_BLEND_CK_MAX(x)(0x10c0 + 0x04 * (x)) > > -#define SUN8I_MIXER_BLEND_CK_MIN(x)(0x10e0 + 0x04 * (x)) > > -#define SUN8I_MIXER_BLEND_OUTCTL 0x10fc > > +#define DE2_BLD_BASE 0x1000 > > +#define DE2_CH_BASE0x2000 > > +#define DE2_CH_SIZE0x1000 > > + > > +#define SUN8I_MIXER_BLEND_PIPE_CTL(base) ((base) + 0) > > +#define SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, x) ((base) + 0x4 + 0x10 * > > (x)) > > +#define SUN8I_MIXER_BLEND_ATTR_INSIZE(base, x) ((base) + 0x8 + 0x10 * > > (x)) > > +#define SUN8I_MIXER_BLEND_ATTR_COORD(base, x) ((base) + 0xC + 0x10 * > > (x)) > > Nit: Use lowercase for '0xC' to be consistent. > > > +#define SUN8I_MIXER_BLEND_ROUTE(base) ((base) + 0x80) > > +#define SUN8I_MIXER_BLEND_PREMULTIPLY(base)((base) + 0x84) > > +#define SUN8I_MIXER_BLEND_BKCOLOR(base)((base) + 0x88) > > +#define SUN8I_MIXER_BLEND_OUTSIZE(base)((base) + 0x8c) > > +#define SUN8I_MIXER_BLEND_MODE(base, x)((base) + 0x90 + > > 0x04 * (x)) +#define SUN8I_MIXER_BLEND_CK_CTL(base) ((base) + > > 0xb0) > > +#define SUN8I_MIXER_BLEND_CK_CFG(base) ((base) + 0xb4) > > +#define SUN8I_MIXER_BLEND_CK_MAX(base, x) ((base) + 0xc0 + 0x04 * > > (x)) +#define SUN8I_MIXER_BLEND_CK_MIN(base, x) ((base) + 0xe0 + > > 0x04 * (x)) +#define SUN8I_MIXER_BLEND_OUTCTL(base) ((base) + > > 0xfc) > > > > #define SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK GENMASK(12, 8) > > #define SUN8I_MIXER_BLEND_PIPE_CTL_EN(pipe)BIT(8 + pipe) > > [...] > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_scaler.c > > b/drivers/gpu/drm/sun4i/sun8i_ui_scaler.c index > > 6bb2aa164c8e..c68eab8a748f 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_ui_scaler.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_scaler.c > > @@ -10,6 +10,7 @@ > > > > */ > > > > #include "sun8i_ui_scaler.h" > > > > +#include "sun8i_vi_scaler.h" > > > > static const u32 lan2coefftab16[240] = { > > > > 0x4000, 0x00033ffe, 0x00063efc, 0x000a3bfb, > > > > @@ -88,6 +89,14 @@ static const u32 lan2coefftab16[240] = { > > > > 0x0b1c1603, 0x0d1c1502, 0x0e1d1401, 0x0f1d1301, > > > > }; > > > > +static inline u32 sun8i_ui_scaler_base(struct sun8i_mixer *mixer, int > > channel) > I recently saw a review comment stating one should not inline functions > unless they are defined in header files. Otherwise the decision should > be left up to the compiler. > > > +{ > > + int vi_num = mixer->cfg->vi_num; > > + > > + return DE2_VI_SCALER_BASE + DE2_VI_SCALER_SIZE * vi_num + > > + DE2_UI_SCALER_SIZE * (channel - vi_num); > > +} > > + > > > > static int sun8i_ui_scaler_coef_index(unsigned int step) > > { > > > > unsigned int scale, int_part, float_part; > > [...] > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c > > b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c index > > d3f1acb234b7..8697afc36023 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c > > @@ -833,6 +833,11 @@ static const u32 bicubic4coefftab32[480] = { > > > > 0x1012110d, 0x1012110d, 0x1013110c, 0x1013110c, > > > > }; > > > > +static inline u32 sun8i_vi_scaler_base(struct sun8i_mixer *mixer, int >
[Bug 108037] Turning monitors off and on again makes the kernel panic and system freeze
https://bugs.freedesktop.org/show_bug.cgi?id=108037 Michel Dänzer changed: What|Removed |Added Product|xorg|DRI Assignee|xorg-driver-...@lists.x.org |dri-devel@lists.freedesktop ||.org Version|git |unspecified Component|Driver/AMDgpu |DRM/AMDgpu QA Contact|xorg-t...@lists.x.org | -- 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] drm/fb_helper: Allow leaking fbdev smem_start
Hi Maxime, Daniel, On 22/09/2018 10:56, Daniel Vetter wrote: > On Fri, Sep 21, 2018 at 04:07:40PM +0200, Maxime Ripard wrote: >> Hi, >> >> On Thu, Sep 20, 2018 at 03:04:12PM +0200, Neil Armstrong wrote: >>> Since "drm/fb: Stop leaking physical address", the default behaviour of >>> the DRM fbdev emulation is to set the smem_base to 0 and pass the new >>> FBINFO_HIDE_SMEM_START flag. >>> >>> The main reason is to avoid leaking physical addresse to user-space, and >>> it follows a general move over the kernel code to avoid user-space to >>> manipulate physical addresses and then use some other mechanisms like >>> dma-buf to transfer physical buffer handles over multiple subsystems. >>> >>> But, a lot of devices depends on closed sources binaries to enable >>> OpenGL hardware acceleration that uses this smem_start value to >>> pass physical addresses to out-of-tree modules in order to render >>> into these physical adresses. These should use dma-buf buffers allocated >>> from the DRM display device instead and stop relying on fbdev overallocation >>> to gather DMA memory (some HW vendors delivers GBM and Wayland capable >>> binaries, but older unsupported devices won't have these new binaries >>> and are doomed until an Open Source solution like Lima finalizes). >>> >>> Since these devices heavily depends on this kind of software and because >>> the smem_start population was available for years, it's a breakage to >>> stop leaking smem_start without any alternative solutions. >>> >>> This patch adds a Kconfig depending on the EXPERT config and an unsafe >>> kernel module parameter tainting the kernel when enabled. >>> >>> A clear comment and Kconfig help text was added to clarify why and when >>> this patch should be reverted, but in the meantime it's a necessary >>> feature to keep. >>> >>> Cc: Dave Airlie >>> Cc: Daniel Vetter >>> Cc: Bartlomiej Zolnierkiewicz >>> Cc: Noralf Trønnes >>> Cc: Maxime Ripard >>> Cc: Eric Anholt >>> Cc: Lucas Stach >>> Cc: Rob Clark >>> Cc: Ben Skeggs >>> Cc: Christian König >>> Signed-off-by: Neil Armstrong >>> --- >>> drivers/gpu/drm/Kconfig | 15 +++ >>> drivers/gpu/drm/drm_fb_helper.c | 33 +++-- >>> 2 files changed, 46 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>> index 8871b3f..b07c298 100644 >>> --- a/drivers/gpu/drm/Kconfig >>> +++ b/drivers/gpu/drm/Kconfig >>> @@ -110,6 +110,21 @@ config DRM_FBDEV_OVERALLOC >>> is 100. Typical values for double buffering will be 200, >>> triple buffering 300. >>> >>> +config DRM_FBDEV_LEAK_PHYS_SMEM >>> + bool "Shamelessly allow leaking of fbdev physical address (DANGEROUS)" >>> + depends on DRM_FBDEV_EMULATION && EXPERT >>> + default n >>> + help >>> + In order to keep user-space compatibility, we want in certain >>> + use-cases to keep leaking the fbdev physical address to the >>> + user-space program handling the fbdev buffer. >>> + This option is a shame and should be removed as soon as possible >>> + and be considered as a broken and legacy behaviour from a modern >>> + fbdev device driver. > > s/shame/not supported by upstream developers/ Ack > > And maybe add "Please send any bug reports when using this to your > proprietary software vendor that requires this." It's (somehow) on the following line : >>> + If in doubt, say "N" or spread the word to your closed source >>> + library vendor. I will make it even scarier ! > > I'd also be in favour of just calling this the ARM Mali blob on > $affected_devices. Makes it easier for people to find the magic knob > they're looking for, if they're unlucky enough to have such a soc. I'm not sure it's only for Mali... Anyway I'll add "Mali" to help people findind it. > >>> + >>> + If in doubt, say "N" or spread the word to your closed source >>> + library vendor. >>> + >>> config DRM_LOAD_EDID_FIRMWARE >>> bool "Allow to specify an EDID data set instead of probing for it" >>> depends on DRM >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c >>> b/drivers/gpu/drm/drm_fb_helper.c >>> index bf2190c..a354944 100644 >>> --- a/drivers/gpu/drm/drm_fb_helper.c >>> +++ b/drivers/gpu/drm/drm_fb_helper.c >>> @@ -56,6 +56,25 @@ MODULE_PARM_DESC(drm_fbdev_overalloc, >>> "Overallocation of the fbdev buffer (%) [default=" >>> __MODULE_STRING(CONFIG_DRM_FBDEV_OVERALLOC) "]"); >>> >>> +/* >>> + * In order to keep user-space compatibility, we want in certain use-cases >>> + * to keep leaking the fbdev physical address to the user-space program >>> + * handling the fbdev buffer. >>> + * This is a bad habit essentially kept into closed source opengl driver >>> + * that should really be moved into open-source upstream projects instead >>> + * of using legacy physical addresses in user space to communicate with >>> + * other out-of-tree kernel modules. >>> + * >>> + * This module_param *s
[Bug 108020] Screen artifacting with 4.18+ kernels on AMD Radeon Vega 64 & 1440p 120hz+ monitors
https://bugs.freedesktop.org/show_bug.cgi?id=108020 Michel Dänzer changed: What|Removed |Added CC||harry.wentl...@amd.com, ||nicholas.kazlaus...@amd.com ||, sunpeng...@amd.com --- Comment #1 from Michel Dänzer --- Please attach the dmesg output. -- 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 108019] Gothic 2 under wine does not redraw in fullscreen
https://bugs.freedesktop.org/show_bug.cgi?id=108019 --- Comment #1 from Michel Dänzer --- Can you try Mesa 18.2.1? It sounds like https://gitlab.freedesktop.org/mesa/mesa/commit/e4b667224d6ddd5d42a1349729337ce68a1afca9 might help. -- 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 107940] libdrm 2.4.94-1 causes blender to crash on opencl
https://bugs.freedesktop.org/show_bug.cgi?id=107940 Michel Dänzer changed: What|Removed |Added CC||ckoenig.leichtzumerken@gmai ||l.com --- Comment #6 from Michel Dänzer --- (In reply to Mowley from comment #5) > I'm guessing that my XOrg log file would now be superfluous? Yeah, actually comment 3 was intended for another bug report, sorry. Christian, any idea how the commit below could cause this? commit 7aa1a511336dd7cb26dafef81d76edd6978a6cdf Author: Christian König Date: Wed Aug 1 20:44:44 2018 +0200 amdgpu: stop using the hash table for fd_tab -- 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 107940] libdrm 2.4.94-1 causes blender to crash on opencl
https://bugs.freedesktop.org/show_bug.cgi?id=107940 --- Comment #7 from Christian König --- (In reply to Michel Dänzer from comment #6) > Christian, any idea how the commit below could cause this? -EBADF means the file descriptor number is already closed. So maybe the application is trying to open/close the device multiple times and we have a bug in amdgpu_device_free_internal? 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 02/10] phy: Add configuration interface
Hi, On Mon, Sep 24, 2018 at 02:18:35PM +0530, Kishon Vijay Abraham I wrote: > On Wednesday 19 September 2018 05:44 PM, Maxime Ripard wrote: > > Hi, > > > > On Fri, Sep 14, 2018 at 02:18:37PM +0530, Kishon Vijay Abraham I wrote: > >>> +/** > >>> + * 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. > > Maybe the PHY should provide the list of supported features to the > consumer > driver and the consumer should select a supported feature? > >>> > >>> It's not really about the features it supports, but the boundaries it > >>> might have on those features. For example, the same phy integrated in > >>> two different SoCs will probably have some limit on the clock rate it > >>> can output because of the phy design itself, but also because of the > >>> clock that is fed into that phy, and that will be different from one > >>> SoC to the other. > >>> > >>> This integration will prevent us to use some clock rates on the first > >>> SoC, while the second one would be totally fine with it. > >> > >> If there's a clock that is fed to the PHY from the consumer, then the > >> consumer > >> driver should model a clock provider and the PHY can get a reference to it > >> using clk_get(). Rockchip and Arasan eMMC PHYs has already used something > >> like > >> that. > > > > That would be doable, but no current driver has had this in their > > binding. So that would prevent any further rework, and make that whole > > series moot. And while I could live without the Allwinner part, the > > Cadence one is really needed. > > We could add a binding and modify the driver to to register a clock provider. > That could be included in this series itself. That wouldn't work for device whose bindings need to remain backward compatible. And the Allwinner part at least is in that case. I think we should aim at making it a norm for newer bindings, but we still have to support the old ones that cannot be changed. > >> Assuming the PHY can get a reference to the clock provided by the consumer, > >> what are the parameters we'll be able to get rid of in struct > >> phy_configure_opts_mipi_dphy? > > > > hs_clock_rate and lp_clock_rate. All the other ones are needed. > > For a start we could use that and get rid of hs_clock_rate and lp_clock_rate > in > phy_configure_opts_mipi_dphy. As I was saying above, I'm not sure we can do that. > We could also use phy_set_bus_width() for lanes. I overlooked this function somehow, it indeed looks like we can remove the lanes part in favor of this function. > > > >> I'm sorry but I'm not convinced a consumer driver should have all the > >> details > >> that are added in phy_configure_opts_mipi_dphy. > > > > If it can convince you, here is the parameters that are needed by all > > the MIPI-DSI drivers currently in Linux to configure their PHY: > > > > - cdns-dsi (drivers/gpu/drm/bridge/cdns-dsi.c) > > - hs_clk_rate > > - lanes > > - videomode > > > > - kirin (drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c) > > - hs_exit > > - hs_prepare > > - hs_trail > > - hs_zero > > - lpx > > - ta_get > > - ta_go > > - wakeup > > > > - msm (drivers/gpu/drm/msm/dsi/*) > > - clk_post > > - clk_pre > > - clk_prepare > > - clk_trail > > - clk_zero > > - hs_clk_rate > > - hs_exit > > - hs_prepare > > - hs_trail > > - hs_zero > > - lp_clk_rate > > - ta_get > > - ta_go > > - ta_sure > > > > - mtk (drivers/gpu/drm/mediatek/mtk_dsi.c) > > - hs_clk_rate
[Bug 108036] VA-API implementation reports support for unsupported endpoints
https://bugs.freedesktop.org/show_bug.cgi?id=108036 --- Comment #1 from Christian König --- The problem is neither the driver nor the application, but rather the design of VA-API. The hardware supports some high profile features (like CABAC), but unfortunately not all of them (like B-frames or MBAFF). Now the profile selects what the decoder needs to be able to do to handle a certain video, but doesn't tells you anything about the encoder except for selecting the encoding of the headers. We should support the encoding of the headers, so if an application selects high profile it actually gets better compression because of CABAC support. But if the application also tries to use B-frames it will get an invalid stream. Additional to that we currently have a firmware problem which corrupts all streams != baseline. Could be that you are running into that one. Boyuang is already investigating it and you could try to downgrade your firmware version to test that. -- 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 108019] Gothic 2 under wine does not redraw in fullscreen
https://bugs.freedesktop.org/show_bug.cgi?id=108019 --- Comment #2 from supercoolem...@seznam.cz --- (In reply to Michel Dänzer from comment #1) > Can you try Mesa 18.2.1? It sounds like > https://gitlab.freedesktop.org/mesa/mesa/commit/ > e4b667224d6ddd5d42a1349729337ce68a1afca9 might help. It is really fixed in 18.2.1. Thank you. -- 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 4/6] drm: add timeline syncobj payload query ioctl
Am 20.09.2018 um 13:03 schrieb Chunming Zhou: user mode can query timeline payload. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/drm_internal.h | 2 ++ drivers/gpu/drm/drm_ioctl.c| 2 ++ drivers/gpu/drm/drm_syncobj.c | 53 ++ include/uapi/drm/drm.h | 11 +++ 4 files changed, 68 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 566d44e3c782..9c4826411a3c 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -189,6 +189,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private); +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private); /* drm_framebuffer.c */ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index c0891614f516..c3c0617e6372 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -675,6 +675,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl, + DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index a43de0e4616c..e998fc319f9f 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1289,3 +1289,56 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data, return ret; } + +static void drm_syncobj_timeline_query_payload(struct drm_syncobj *syncobj, + uint64_t *point) +{ + if (syncobj->type != DRM_SYNCOBJ_TYPE_TIMELINE) { + DRM_ERROR("Normal syncobj cann't be queried!"); + *point = 0; + return; + } + *point = syncobj->signal_point; +} + +int drm_syncobj_query_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_private) +{ + struct drm_syncobj_timeline_query *args = data; + struct drm_syncobj **syncobjs; + uint64_t *points; + uint32_t i; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + return -ENODEV; + + if (args->count_handles == 0) + return -EINVAL; + + ret = drm_syncobj_array_find(file_private, +u64_to_user_ptr(args->handles), +args->count_handles, +&syncobjs); + if (ret < 0) + return ret; + + + points = kmalloc_array(args->count_handles, sizeof(*points), + GFP_KERNEL); + if (points == NULL) { + ret = -ENOMEM; + goto out; + } + for (i = 0; i < args->count_handles; i++) { + drm_syncobj_timeline_query_payload(syncobjs[i], &points[i]); + copy_to_user(u64_to_user_ptr(args->points), points, +sizeof(uint64_t) * args->count_handles); Copying the points to userspace on each iteration of the loop? That doesn't looks correct to me. Additional to that you need to check the return value of copy_to_user. Regards, Christian. + } + + kfree(points); +out: + drm_syncobj_array_free(syncobjs, args->count_handles); + + return ret; +} diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 501e86d81f47..188b63f1975b 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -767,6 +767,15 @@ struct drm_syncobj_array { __u32 pad; }; +struct drm_syncobj_timeline_query { + __u64 handles; + /* points are timeline syncobjs payloads returned by query ioctl */ + __u64 points; + __u32 count_handles; + __u32 pad; +}; + + /* Query current scanout sequence number */ struct drm_crtc_get_sequence { __u32 crtc_id; /* requested crtc_id */ @@ -924,6 +933,8 @@ extern "C" { #define DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct drm_mode_revoke_lease) #define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait) +#define DRM_IOCTL_SYNCOBJ_QUERYDRM_IOWR(0xCB, struct drm_synco
[Bug 108019] Gothic 2 under wine does not redraw in fullscreen
https://bugs.freedesktop.org/show_bug.cgi?id=108019 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Michel Dänzer --- (In reply to supercoolemail from comment #2) > It is really fixed in 18.2.1. Thank you. Excellent, thanks for testing and following up. -- 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 5/6] drm/amdgpu: add timeline support in amdgpu CS
Am 20.09.2018 um 13:03 schrieb Chunming Zhou: syncobj wait/signal operation is appending in command submission. Signed-off-by: Chunming Zhou --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 8 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 114 +++-- include/uapi/drm/amdgpu_drm.h | 10 +++ 3 files changed, 104 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 447c4c7a36d6..6e4a3db56833 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -975,6 +975,11 @@ struct amdgpu_cs_chunk { void*kdata; }; +struct amdgpu_cs_syncobj_post_dep { + struct drm_syncobj *post_dep_syncobj; + u64 point; +}; + struct amdgpu_cs_parser { struct amdgpu_device*adev; struct drm_file *filp; @@ -1003,9 +1008,8 @@ struct amdgpu_cs_parser { /* user fence */ struct amdgpu_bo_list_entry uf_entry; - + struct amdgpu_cs_syncobj_post_dep *post_dep_syncobjs; unsigned num_post_dep_syncobjs; - struct drm_syncobj **post_dep_syncobjs; }; static inline u32 amdgpu_get_ib_value(struct amdgpu_cs_parser *p, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 412fac238575..0efe75bf2f03 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -204,6 +204,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs case AMDGPU_CHUNK_ID_DEPENDENCIES: case AMDGPU_CHUNK_ID_SYNCOBJ_IN: case AMDGPU_CHUNK_ID_SYNCOBJ_OUT: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT: + case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL: break; default: @@ -783,7 +785,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, &parser->validated); for (i = 0; i < parser->num_post_dep_syncobjs; i++) - drm_syncobj_put(parser->post_dep_syncobjs[i]); + drm_syncobj_put(parser->post_dep_syncobjs[i].post_dep_syncobj); kfree(parser->post_dep_syncobjs); dma_fence_put(parser->fence); @@ -1098,11 +1100,13 @@ static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p, } static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, -uint32_t handle) +uint32_t handle, u64 point, +u64 flags) { int r; struct dma_fence *fence; - r = drm_syncobj_find_fence(p->filp, handle, 0, 0, &fence); + + r = drm_syncobj_find_fence(p->filp, handle, point, flags, &fence); if (r) return r; @@ -1113,48 +1117,91 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, } static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p, - struct amdgpu_cs_chunk *chunk) + struct amdgpu_cs_chunk *chunk, + bool timeline) You should really just duplicate the function. The only think you have in common here is allocating the post_dep_sync objects. Would also prevent that we need to make the two chunk types mutual exclusive. Christian. { unsigned num_deps; int i, r; - struct drm_amdgpu_cs_chunk_sem *deps; - deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; - num_deps = chunk->length_dw * 4 / - sizeof(struct drm_amdgpu_cs_chunk_sem); + if (!timeline) { + struct drm_amdgpu_cs_chunk_sem *deps; - for (i = 0; i < num_deps; ++i) { - r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle); + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_sem); + for (i = 0; i < num_deps; ++i) { + r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle, + 0, 0); if (r) return r; + } + } else { + struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps; + + syncobj_deps = (struct drm_amdgpu_cs_chunk_syncobj *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_syncobj); + for (i = 0; i < num_deps; ++i) { + r = amdgpu_syncobj_lookup_and_add_to_sync(p, syncobj_deps[i].handle, + syn
Re: [PATCH v2 09/16] drm: rcar-du: Cache DSYSR value to ensure known initial value
Hi Laurent, On 14/09/18 10:10, Laurent Pinchart wrote: > DSYSR is a DU channel register that also contains group fields. It is > thus written to by both the group and CRTC code, using read-update-write > sequences. As the register isn't initialized explicitly at startup time, > this can lead to invalid or otherwise unexpected values being written to > some of the fields if they have been modified by the firmware or just > not reset properly. > > To fix this we can write a fully known value to the DSYSR register when > turning a channel's functional clock on. However, the mix of group and > channel fields complicate this. A simpler solution is to cache the > register and initialize the cached value to the desired hardware > defaults. Great, this looks good to me, and is less overhead than pulling in a full reg-cache when we only need a single register handled. > Signed-off-by: Laurent Pinchart > Tested-by: Jacopo Mondi Reviewed-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 16 > drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 5 + > drivers/gpu/drm/rcar-du/rcar_du_group.c | 7 --- > 3 files changed, 17 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index 2f8776c1ec8f..f827fccf6416 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -57,13 +57,12 @@ static void rcar_du_crtc_set(struct rcar_du_crtc *rcrtc, > u32 reg, u32 set) > rcar_du_read(rcdu, rcrtc->mmio_offset + reg) | set); > } > > -static void rcar_du_crtc_clr_set(struct rcar_du_crtc *rcrtc, u32 reg, > - u32 clr, u32 set) > +void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set) > { > struct rcar_du_device *rcdu = rcrtc->group->dev; > - u32 value = rcar_du_read(rcdu, rcrtc->mmio_offset + reg); > > - rcar_du_write(rcdu, rcrtc->mmio_offset + reg, (value & ~clr) | set); > + rcrtc->dsysr = (rcrtc->dsysr & ~clr) | set; > + rcar_du_write(rcdu, rcrtc->mmio_offset + DSYSR, rcrtc->dsysr); > } > > /* > - > @@ -576,9 +575,9 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc) >* actively driven). >*/ > interlaced = rcrtc->crtc.mode.flags & DRM_MODE_FLAG_INTERLACE; > - rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK | DSYSR_SCM_MASK, > - (interlaced ? DSYSR_SCM_INT_VIDEO : 0) | > - DSYSR_TVM_MASTER); > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK | DSYSR_SCM_MASK, > +(interlaced ? DSYSR_SCM_INT_VIDEO : 0) | > +DSYSR_TVM_MASTER); > > rcar_du_group_start_stop(rcrtc->group, true); > } > @@ -645,7 +644,7 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) >* Select switch sync mode. This stops display operation and configures >* the HSYNC and VSYNC signals as inputs. >*/ > - rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH); > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH); > > rcar_du_group_start_stop(rcrtc->group, false); > } > @@ -1121,6 +1120,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, > unsigned int swindex, > rcrtc->group = rgrp; > rcrtc->mmio_offset = mmio_offsets[hwindex]; > rcrtc->index = hwindex; > + rcrtc->dsysr = (rcrtc->index % 2 ? 0 : DSYSR_DRES) | DSYSR_TVM_TVSYNC; > > if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) > primary = &rcrtc->vsp->planes[rcrtc->vsp_pipe].plane; > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > index 4990bbe9ba26..59ac6e7d22c9 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > @@ -30,6 +30,7 @@ struct rcar_du_vsp; > * @mmio_offset: offset of the CRTC registers in the DU MMIO block > * @index: CRTC software and hardware index > * @initialized: whether the CRTC has been initialized and clocks enabled > + * @dsysr: cached value of the DSYSR register > * @vblank_enable: whether vblank events are enabled on this CRTC > * @event: event to post when the pending page flip completes > * @flip_wait: wait queue used to signal page flip completion > @@ -50,6 +51,8 @@ struct rcar_du_crtc { > unsigned int index; > bool initialized; > > + u32 dsysr; > + > bool vblank_enable; > struct drm_pending_vblank_event *event; > wait_queue_head_t flip_wait; > @@ -103,4 +106,6 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc, > enum rcar_du_output output); > void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc); > > +void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcr
Re: [PATCH v2 10/16] drm: rcar-du: Don't use TV sync mode when not supported by the hardware
Hi Laurent, On 14/09/18 10:10, Laurent Pinchart wrote: > The official way to stop the display is to clear the display enable > (DEN) bit in the DSYSR register, but that operates at a group level and > affects the two channels in the group. To disable channels selectively, > the driver uses TV sync mode that stops display operation on the channel > and turns output signals into inputs. > > While TV sync mode is available in all DU models currently supported, > the D3 and E3 DUs don't support it. We will thus need to find an > alternative way to turn channels off. > > In the meantime, condition the switch to TV sync mode to the > availability of the feature, to avoid writing an invalid value to the > DSYSR register. (Perhaps for leading into the next sentence) When the feature is unavailable ... > The display output will turn blank as all planes are > disabled when stopping the CRTC. Otherwise your sentence sounds like it will affect existing platforms. > > Signed-off-by: Laurent Pinchart > Tested-by: Jacopo Mondi But that's a minor and possibly not relevant nit so: Reviewed-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 7 ++- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 33 ++--- > drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + > 3 files changed, 29 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index f827fccf6416..17741843cf51 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -643,8 +643,13 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) > /* >* Select switch sync mode. This stops display operation and configures >* the HSYNC and VSYNC signals as inputs. > + * > + * TODO: Find another way to stop the display for DUs that don't support > + * TVM sync. >*/ > - rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH); > + if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_TVM_SYNC)) > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK, > +DSYSR_TVM_SWITCH); > > rcar_du_group_start_stop(rcrtc->group, false); > } > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index 0954ecd2f943..fa0d381c2d0f 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -36,7 +36,8 @@ static const struct rcar_du_device_info > rzg1_du_r8a7743_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -58,7 +59,8 @@ static const struct rcar_du_device_info > rzg1_du_r8a7745_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -77,7 +79,8 @@ static const struct rcar_du_device_info > rzg1_du_r8a7745_info = { > > static const struct rcar_du_device_info rcar_du_r8a7779_info = { > .gen = 2, > - .features = RCAR_DU_FEATURE_INTERLACED, > + .features = RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -99,7 +102,8 @@ static const struct rcar_du_device_info > rcar_du_r8a7790_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .quirks = RCAR_DU_QUIRK_ALIGN_128B, > .channels_mask = BIT(2) | BIT(1) | BIT(0), > .routes = { > @@ -128,7 +132,8 @@ static const struct rcar_du_device_info > rcar_du_r8a7791_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > .channels_mask = BIT(1) | BIT(0), > .routes = { > /* > @@ -151,7 +156,8 @@ static const struct rcar_du_device_info > rcar_du_r8a7792_info = { > .gen = 2, > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > | RCAR_DU_FEATURE_EXT_CTRL_REGS > - | RCAR_DU_FEATURE_INTERLACED, > + | RCAR_DU_FEATURE_INTERLACED > + | R
Re: [PATCH v2 02/16] dt-bindings: display: renesas: lvds: Document r8a77990 bindings
Hi Laurent, On 14/09/18 10:10, Laurent Pinchart wrote: > The E3 (r8a77990) supports two LVDS channels. Extend the binding to > support them. This commit message sounds rather like we are modifying the bindings, rather than just adding a compatible string. I went looking for extra changes - but I see we only need a compatible addition. > Signed-off-by: Laurent Pinchart > Reviewed-by: Jacopo Mondi Still, the change looks accurate to me - and at the moment I can't think of a better description so: Reviewed-by: Kieran Bingham > --- > Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > index 5a4e379bb414..13af7e2ac7e8 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > @@ -15,6 +15,7 @@ Required properties: >- "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders >- "renesas,r8a77970-lvds" for R8A77970 (R-Car V3M) compatible LVDS encoders >- "renesas,r8a77980-lvds" for R8A77980 (R-Car V3H) compatible LVDS encoders > + - "renesas,r8a77990-lvds" for R8A77990 (R-Car E3) compatible LVDS encoders >- "renesas,r8a77995-lvds" for R8A77995 (R-Car D3) compatible LVDS encoders > > - reg: Base address and length for the memory-mapped registers > -- Regards -- Kieran ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/16] drm: rcar-du: Add r8a77990 and r8a77995 device support
Hi Laurent, Uli, On 14/09/18 10:10, Laurent Pinchart wrote: > From: Ulrich Hecht > > Add support for the R-Car D3 (R8A77995) and E3 (R8A77990) SoCs to the > R-Car DU driver. The two SoCs instantiate compatible DUs, so a single > information structure is enough. > > Signed-off-by: Ulrich Hecht > [Add support for R8A77990] > Signed-off-by: Laurent Pinchart > Tested-by: Jacopo Mondi Reviewed-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 30 ++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index fa0d381c2d0f..084f58df4a8c 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -311,6 +311,34 @@ static const struct rcar_du_device_info > rcar_du_r8a77970_info = { > .num_lvds = 1, > }; > > +static const struct rcar_du_device_info rcar_du_r8a7799x_info = { > + .gen = 3, > + .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK > + | RCAR_DU_FEATURE_EXT_CTRL_REGS > + | RCAR_DU_FEATURE_VSP1_SOURCE, > + .channels_mask = BIT(1) | BIT(0), > + .routes = { > + /* > + * R8A77990 and R8A77995 have one RGB output and two LVDS > + * outputs. > + */ > + [RCAR_DU_OUTPUT_DPAD0] = { > + .possible_crtcs = BIT(0) | BIT(1), > + .port = 0, > + }, > + [RCAR_DU_OUTPUT_LVDS0] = { > + .possible_crtcs = BIT(0), > + .port = 1, > + }, > + [RCAR_DU_OUTPUT_LVDS1] = { > + .possible_crtcs = BIT(1), > + .port = 2, > + }, > + }, > + .num_lvds = 2, > + .lvds_clk_mask = BIT(1) | BIT(0), > +}; > + > static const struct of_device_id rcar_du_of_table[] = { > { .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info }, > { .compatible = "renesas,du-r8a7745", .data = &rzg1_du_r8a7745_info }, > @@ -324,6 +352,8 @@ static const struct of_device_id rcar_du_of_table[] = { > { .compatible = "renesas,du-r8a7796", .data = &rcar_du_r8a7796_info }, > { .compatible = "renesas,du-r8a77965", .data = &rcar_du_r8a77965_info }, > { .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info }, > + { .compatible = "renesas,du-r8a77990", .data = &rcar_du_r8a7799x_info }, > + { .compatible = "renesas,du-r8a77995", .data = &rcar_du_r8a7799x_info }, > { } > }; > > -- Regards -- Kieran ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH 2/2] drm: Use new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags
On Sat, Sep 22, 2018 at 03:15:04PM +0300, Laurent Pinchart wrote: > The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE flags are deprecated in favour of > the new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace > them through the code. > > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/bridge/dumb-vga-dac.c| 6 +++--- > drivers/gpu/drm/drm_modes.c | 8 > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- > drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +- > drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 6 +++--- > drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c| 2 +- > .../drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c | 2 +- > .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-sony-acx565akm.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c | 2 +- > drivers/gpu/drm/omapdrm/dss/dsi.c| 2 +- > drivers/gpu/drm/omapdrm/dss/sdi.c| 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c | 4 ++-- > drivers/gpu/drm/panel/panel-arm-versatile.c | 4 ++-- > drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 4 ++-- > drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 2 +- > drivers/gpu/drm/panel/panel-simple.c | 20 > ++-- > drivers/gpu/drm/pl111/pl111_display.c| 2 +- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- > drivers/gpu/drm/tve200/tve200_display.c | 3 ++- > include/drm/drm_bridge.h | 8 > 23 files changed, 47 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c > b/drivers/gpu/drm/bridge/dumb-vga-dac.c > index 9b706789a341..7dc14c22f7db 100644 > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c > @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev) > */ > static const struct drm_bridge_timings default_dac_timings = { > /* Timing specifications, datasheet page 7 */ > - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, > + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > .setup_time_ps = 500, > .hold_time_ps = 1500, > }; > @@ -245,7 +245,7 @@ static const struct drm_bridge_timings > default_dac_timings = { > */ > static const struct drm_bridge_timings ti_ths8134_dac_timings = { > /* From timing diagram, datasheet page 9 */ > - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, > + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > /* From datasheet, page 12 */ > .setup_time_ps = 3000, > /* I guess this means latched input */ > @@ -258,7 +258,7 @@ static const struct drm_bridge_timings > ti_ths8134_dac_timings = { > */ > static const struct drm_bridge_timings ti_ths8135_dac_timings = { > /* From timing diagram, datasheet page 14 */ > - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, > + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > /* From datasheet, page 16 */ > .setup_time_ps = 2000, > .hold_time_ps = 500, Now I'm confused. Aren't you effectively iverting the sampling edges here? That and the fact that everywhere else we only use the driver variants makes me think that we should just define which way these are supposed to be used and just have a single set of definitions. Also, I think there's no need for these to be always "physically" correct. This is up to interpretation by the driver, so if a bridge driver wants to use them as meaning "sampled edge", that's fine. If display controllers use them to mean "driven edge", that's equally fine. As long as we don't communicate the flags between drivers there should be no reason for them to be confusing. Just make sure that the comments in the interfaces clarify from which point of view these flags are to be interpreted and we should be fine. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: Fix a buffer object eviction regression v2
Commit 4eb085e42fde ("drm/vmwgfx: Convert to new IDA API") introduced an incorrect return value from the function vmw_gmrid_man_get_node(), when we run out of integer ids. Instead of returning 0 (meaning non-fatal error) we forward the ida_simple_get error code -ENOSPC. This causes TTM not to retry allocation after buffer eviction and instead return -ENOSPC to user-space. Fix this by returning 0 when ida_simple_get() returns other error codes than -ENOMEM: Tested using glretrace. Cc: Signed-off-by: Thomas Hellstrom Reviewed-by: Sinclair Yeh --- v2: Test for -ENOMEM instead of -ENOSPC --- drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c index b93c558dd86e..c1cdf6975396 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c @@ -57,7 +57,7 @@ static int vmw_gmrid_man_get_node(struct ttm_mem_type_manager *man, id = ida_alloc_max(&gman->gmr_ida, gman->max_gmr_ids - 1, GFP_KERNEL); if (id < 0) - return id; + return (id == -ENOMEM ? -ENOMEM : 0); spin_lock(&gman->lock); -- 2.19.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH 0/2] Clarify display info PIXDATA bus flags
On 22.09.2018 14:15, Laurent Pinchart wrote: > Hello, > > This patch series attemps at clarifying usage of the > DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE flags. It results from a discussion > on the mailing list available at [1]. > > The problem being discussed was confusion around how the > DRM_BUS_FLAG_PIXDATA_POSEDGE and DRM_BUS_FLAG_PIXDATA_NEGEDGE flags > could be interpreted (and are interpreted now by drivers). Patch 1/2 > introduces new, more explicit flags, and explains the rationale. Patch > 2/2 then updates the drivers to use the new flags. IMHO, the meaning was quite clearly stated... I am not sure whether this added clarity is worth the churn. But I am ok with it if others think it's necessary. Btw, if we change this for DRM_BUS_FLAG*, we probably should also do the equal change for DISPLAY_FLAGS_PIXDATA_[NEG|POS]EDGE. Since displays are always on the sample side, it probably has higher chance to get mixed up. -- Stefan > > [1] https://www.spinics.net/lists/arm-kernel/msg677079.html > > Laurent Pinchart (2): > drm: Clarify definition of the DRM_BUS_FLAG_PIXDATA_* macros > drm: Use new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags > > drivers/gpu/drm/bridge/dumb-vga-dac.c| 6 +++--- > drivers/gpu/drm/drm_modes.c | 8 > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- > drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +- > drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 6 +++--- > drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c| 2 +- > .../drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c | 2 +- > .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-sony-acx565akm.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 2 +- > .../gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c | 2 +- > drivers/gpu/drm/omapdrm/dss/dsi.c| 2 +- > drivers/gpu/drm/omapdrm/dss/sdi.c| 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c | 4 ++-- > drivers/gpu/drm/panel/panel-arm-versatile.c | 4 ++-- > drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 4 ++-- > drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 2 +- > drivers/gpu/drm/panel/panel-simple.c | 20 > ++-- > drivers/gpu/drm/pl111/pl111_display.c| 2 +- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- > drivers/gpu/drm/tve200/tve200_display.c | 3 ++- > include/drm/drm_bridge.h | 8 > include/drm/drm_connector.h | 20 > ++-- > 24 files changed, 65 insertions(+), 48 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Improve reservation object shared slot function
The reservation object shared slot function only allowed to reserve one slot at a time. Improve that and allow to reserve multiple slots to support atomically submission to multiple engines. Please comment and review, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/7] drm/amdgpu: fix using shared fence for exported BOs
It is perfectly possible that the BO list is created before the BO is exported. While at it cleanup setting shared to one instead of true. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 13 ++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index 14d2982a47cc..5c79da8e1150 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -118,7 +118,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, entry->priority = min(info[i].bo_priority, AMDGPU_BO_LIST_MAX_PRIORITY); entry->tv.bo = &bo->tbo; - entry->tv.shared = !bo->prime_shared_count; if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_GDS) list->gds_obj = bo; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 3243da67db9e..1a2ea2e931e3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -50,7 +50,7 @@ static int amdgpu_cs_user_fence_chunk(struct amdgpu_cs_parser *p, bo = amdgpu_bo_ref(gem_to_amdgpu_bo(gobj)); p->uf_entry.priority = 0; p->uf_entry.tv.bo = &bo->tbo; - p->uf_entry.tv.shared = true; + p->uf_entry.tv.shared = 1; p->uf_entry.user_pages = NULL; drm_gem_object_put_unlocked(gobj); @@ -598,6 +598,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, return r; } + amdgpu_bo_list_for_each_entry(e, p->bo_list) + e->tv.shared = 1; + amdgpu_bo_list_get_list(p->bo_list, &p->validated); if (p->bo_list->first_userptr != p->bo_list->num_entries) p->mn = amdgpu_mn_get(p->adev, AMDGPU_MN_TYPE_GFX); @@ -717,8 +720,12 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, gws = p->bo_list->gws_obj; oa = p->bo_list->oa_obj; - amdgpu_bo_list_for_each_entry(e, p->bo_list) - e->bo_va = amdgpu_vm_bo_find(vm, ttm_to_amdgpu_bo(e->tv.bo)); + amdgpu_bo_list_for_each_entry(e, p->bo_list) { + struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); + + e->tv.shared = bo->prime_shared_count ? 1 : 0; + e->bo_va = amdgpu_vm_bo_find(vm, bo); + } if (gds) { p->job->gds_base = amdgpu_bo_gpu_offset(gds) >> PAGE_SHIFT; -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/7] drm/ttm: allow reserve more than one shared slot
Let's support simultaneous submissions to multiple engines. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 6 -- include/drm/ttm/ttm_execbuf_util.h | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index e493edb0d3e7..9b842884530a 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -129,7 +129,8 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, if (!entry->shared) continue; - ret = reservation_object_reserve_shared(bo->resv, 1); + ret = reservation_object_reserve_shared(bo->resv, + entry->shared); if (!ret) continue; } @@ -151,7 +152,8 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, } if (!ret && entry->shared) - ret = reservation_object_reserve_shared(bo->resv, 1); + ret = reservation_object_reserve_shared(bo->resv, + entry->shared); if (unlikely(ret != 0)) { if (ret == -EINTR) diff --git a/include/drm/ttm/ttm_execbuf_util.h b/include/drm/ttm/ttm_execbuf_util.h index b0fdd1980034..3b16dda9bd08 100644 --- a/include/drm/ttm/ttm_execbuf_util.h +++ b/include/drm/ttm/ttm_execbuf_util.h @@ -46,7 +46,7 @@ struct ttm_validate_buffer { struct list_head head; struct ttm_buffer_object *bo; - bool shared; + unsigned int shared; }; /** -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/7] dma-buf: remove shared fence staging in reservation object
No need for that any more. Just replace the list when there isn't enough room any more for the additional fence. Signed-off-by: Christian König --- drivers/dma-buf/reservation.c | 178 ++ include/linux/reservation.h | 4 - 2 files changed, 58 insertions(+), 124 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 6c95f61a32e7..5825fc336a13 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -68,105 +68,23 @@ EXPORT_SYMBOL(reservation_seqcount_string); */ int reservation_object_reserve_shared(struct reservation_object *obj) { - struct reservation_object_list *fobj, *old; - u32 max; + struct reservation_object_list *old, *new; + unsigned int i, j, k, max; old = reservation_object_get_list(obj); if (old && old->shared_max) { - if (old->shared_count < old->shared_max) { - /* perform an in-place update */ - kfree(obj->staged); - obj->staged = NULL; + if (old->shared_count < old->shared_max) return 0; - } else + else max = old->shared_max * 2; - } else - max = 4; - - /* -* resize obj->staged or allocate if it doesn't exist, -* noop if already correct size -*/ - fobj = krealloc(obj->staged, offsetof(typeof(*fobj), shared[max]), - GFP_KERNEL); - if (!fobj) - return -ENOMEM; - - obj->staged = fobj; - fobj->shared_max = max; - return 0; -} -EXPORT_SYMBOL(reservation_object_reserve_shared); - -static void -reservation_object_add_shared_inplace(struct reservation_object *obj, - struct reservation_object_list *fobj, - struct dma_fence *fence) -{ - struct dma_fence *signaled = NULL; - u32 i, signaled_idx; - - dma_fence_get(fence); - - preempt_disable(); - write_seqcount_begin(&obj->seq); - - for (i = 0; i < fobj->shared_count; ++i) { - struct dma_fence *old_fence; - - old_fence = rcu_dereference_protected(fobj->shared[i], - reservation_object_held(obj)); - - if (old_fence->context == fence->context) { - /* memory barrier is added by write_seqcount_begin */ - RCU_INIT_POINTER(fobj->shared[i], fence); - write_seqcount_end(&obj->seq); - preempt_enable(); - - dma_fence_put(old_fence); - return; - } - - if (!signaled && dma_fence_is_signaled(old_fence)) { - signaled = old_fence; - signaled_idx = i; - } - } - - /* -* memory barrier is added by write_seqcount_begin, -* fobj->shared_count is protected by this lock too -*/ - if (signaled) { - RCU_INIT_POINTER(fobj->shared[signaled_idx], fence); } else { - BUG_ON(fobj->shared_count >= fobj->shared_max); - RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); - fobj->shared_count++; + max = 4; } - write_seqcount_end(&obj->seq); - preempt_enable(); - - dma_fence_put(signaled); -} - -static void -reservation_object_add_shared_replace(struct reservation_object *obj, - struct reservation_object_list *old, - struct reservation_object_list *fobj, - struct dma_fence *fence) -{ - unsigned i, j, k; - - dma_fence_get(fence); - - if (!old) { - RCU_INIT_POINTER(fobj->shared[0], fence); - fobj->shared_count = 1; - goto done; - } + new = kmalloc(offsetof(typeof(*new), shared[max]), GFP_KERNEL); + if (!new) + return -ENOMEM; /* * no need to bump fence refcounts, rcu_read access @@ -174,46 +92,45 @@ reservation_object_add_shared_replace(struct reservation_object *obj, * references from the old struct are carried over to * the new. */ - for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) { - struct dma_fence *check; + for (i = 0, j = 0, k = max; i < (old ? old->shared_count : 0); ++i) { + struct dma_fence *fence; - check = rcu_dereference_protected(old->shared[i], - reservation_object_held(obj)); - - if (check->context == fence->context || - dma_fence_is_signaled(check)) - RCU_INIT_POI
[PATCH 2/7] dma-buf: allow reserving more than one shared fence slot
Let's support simultaneous submissions to multiple engines. Signed-off-by: Christian König --- drivers/dma-buf/reservation.c| 13 - drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++-- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- drivers/gpu/drm/i915/i915_vma.c | 2 +- drivers/gpu/drm/msm/msm_gem_submit.c | 3 ++- drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +- drivers/gpu/drm/qxl/qxl_release.c| 2 +- drivers/gpu/drm/radeon/radeon_vm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 4 ++-- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 4 ++-- drivers/gpu/drm/v3d/v3d_gem.c| 2 +- drivers/gpu/drm/vc4/vc4_gem.c| 2 +- drivers/gpu/drm/vgem/vgem_fence.c| 2 +- include/linux/reservation.h | 3 ++- 16 files changed, 28 insertions(+), 23 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 5825fc336a13..5fb4fd461908 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -56,9 +56,10 @@ const char reservation_seqcount_string[] = "reservation_seqcount"; EXPORT_SYMBOL(reservation_seqcount_string); /** - * reservation_object_reserve_shared - Reserve space to add a shared - * fence to a reservation_object. + * reservation_object_reserve_shared - Reserve space to add shared fences to + * a reservation_object. * @obj: reservation object + * @num_fences: number of fences we want to add * * Should be called before reservation_object_add_shared_fence(). Must * be called with obj->lock held. @@ -66,7 +67,8 @@ EXPORT_SYMBOL(reservation_seqcount_string); * RETURNS * Zero for success, or -errno */ -int reservation_object_reserve_shared(struct reservation_object *obj) +int reservation_object_reserve_shared(struct reservation_object *obj, + unsigned int num_fences) { struct reservation_object_list *old, *new; unsigned int i, j, k, max; @@ -74,10 +76,11 @@ int reservation_object_reserve_shared(struct reservation_object *obj) old = reservation_object_get_list(obj); if (old && old->shared_max) { - if (old->shared_count < old->shared_max) + if ((old->shared_count + num_fences) <= old->shared_max) return 0; else - max = old->shared_max * 2; + max = max(old->shared_count + num_fences, + old->shared_max * 2); } else { max = 4; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 8836186eb5ef..3243da67db9e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -955,7 +955,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv); + r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv, 1); if (r) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 904014dc5915..cf768acb51dc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -640,7 +640,7 @@ int amdgpu_bo_backup_to_shadow(struct amdgpu_device *adev, bo_addr = amdgpu_bo_gpu_offset(bo); shadow_addr = amdgpu_bo_gpu_offset(bo->shadow); - r = reservation_object_reserve_shared(bo->tbo.resv); + r = reservation_object_reserve_shared(bo->tbo.resv, 1); if (r) goto err; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 6904d794d60a..bdce05183edb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -772,7 +772,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev, ring = container_of(vm->entity.rq->sched, struct amdgpu_ring, sched); - r = reservation_object_reserve_shared(bo->tbo.resv); + r = reservation_object_reserve_shared(bo->tbo.resv, 1); if (r) return r; @@ -1839,7 +1839,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev, if (r) goto error_free; - r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv); + r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv, 1); if (r) goto error_free; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 983e67f19e45..30875f8f2933 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm
[PATCH 5/7] drm/amdgpu: always reserve two slots for the VM
And drop the now superflous extra reservations. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 1a2ea2e931e3..cda8cf90777c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -962,10 +962,6 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p) if (r) return r; - r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv, 1); - if (r) - return r; - p->job->vm_pd_addr = amdgpu_gmc_pd_addr(vm->root.base.bo); if (amdgpu_vm_debug) { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index bdce05183edb..353367382874 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -616,7 +616,7 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, { entry->priority = 0; entry->tv.bo = &vm->root.base.bo->tbo; - entry->tv.shared = true; + entry->tv.shared = 2; entry->user_pages = NULL; list_add(&entry->tv.head, validated); } @@ -772,10 +772,6 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev, ring = container_of(vm->entity.rq->sched, struct amdgpu_ring, sched); - r = reservation_object_reserve_shared(bo->tbo.resv, 1); - if (r) - return r; - r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx); if (r) goto error; @@ -1839,10 +1835,6 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev, if (r) goto error_free; - r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv, 1); - if (r) - goto error_free; - r = amdgpu_vm_update_ptes(¶ms, start, last + 1, addr, flags); if (r) goto error_free; @@ -3023,6 +3015,10 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, if (r) goto error_free_root; + r = reservation_object_reserve_shared(root->tbo.resv, 1); + if (r) + goto error_unreserve; + r = amdgpu_vm_clear_bo(adev, vm, root, adev->vm_manager.root_level, vm->pte_support_ats); -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/ttm: drop the extra reservation for pipelines BO moves
The driver is now responsible to allocate enough shared slots. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 27 ++- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 461b318ca2a6..7cf6c9d06364 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -873,12 +873,11 @@ EXPORT_SYMBOL(ttm_bo_mem_put); /** * Add the last move fence to the BO and reserve a new shared slot. */ -static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, -struct ttm_mem_type_manager *man, -struct ttm_mem_reg *mem) +static void ttm_bo_add_move_fence(struct ttm_buffer_object *bo, + struct ttm_mem_type_manager *man, + struct ttm_mem_reg *mem) { struct dma_fence *fence; - int ret; spin_lock(&man->move_lock); fence = dma_fence_get(man->move); @@ -886,16 +885,9 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, if (fence) { reservation_object_add_shared_fence(bo->resv, fence); - - ret = reservation_object_reserve_shared(bo->resv, 1); - if (unlikely(ret)) - return ret; - dma_fence_put(bo->moving); bo->moving = fence; } - - return 0; } /** @@ -923,7 +915,8 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, return ret; } while (1); mem->mem_type = mem_type; - return ttm_bo_add_move_fence(bo, man, mem); + ttm_bo_add_move_fence(bo, man, mem); + return 0; } static uint32_t ttm_bo_select_caching(struct ttm_mem_type_manager *man, @@ -992,10 +985,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, bool has_erestartsys = false; int i, ret; - ret = reservation_object_reserve_shared(bo->resv, 1); - if (unlikely(ret)) - return ret; - mem->mm_node = NULL; for (i = 0; i < placement->num_placement; ++i) { const struct ttm_place *place = &placement->placement[i]; @@ -1031,11 +1020,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, return ret; if (mem->mm_node) { - ret = ttm_bo_add_move_fence(bo, man, mem); - if (unlikely(ret)) { - (*man->func->put_node)(man, mem); - return ret; - } + ttm_bo_add_move_fence(bo, man, mem); break; } } -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] drm/amdgpu: always reserve one more shared slot for pipelines BO moves
This allows us to drop the extra reserve in TTM. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index cda8cf90777c..3e31561d141b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -50,7 +50,7 @@ static int amdgpu_cs_user_fence_chunk(struct amdgpu_cs_parser *p, bo = amdgpu_bo_ref(gem_to_amdgpu_bo(gobj)); p->uf_entry.priority = 0; p->uf_entry.tv.bo = &bo->tbo; - p->uf_entry.tv.shared = 1; + p->uf_entry.tv.shared = 2; p->uf_entry.user_pages = NULL; drm_gem_object_put_unlocked(gobj); @@ -599,7 +599,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } amdgpu_bo_list_for_each_entry(e, p->bo_list) - e->tv.shared = 1; + e->tv.shared = 2; amdgpu_bo_list_get_list(p->bo_list, &p->validated); if (p->bo_list->first_userptr != p->bo_list->num_entries) @@ -723,7 +723,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, amdgpu_bo_list_for_each_entry(e, p->bo_list) { struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); - e->tv.shared = bo->prime_shared_count ? 1 : 0; + e->tv.shared = bo->prime_shared_count ? 2 : 0; e->bo_va = amdgpu_vm_bo_find(vm, bo); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 353367382874..96fcb33774b7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -616,7 +616,7 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, { entry->priority = 0; entry->tv.bo = &vm->root.base.bo->tbo; - entry->tv.shared = 2; + entry->tv.shared = 3; entry->user_pages = NULL; list_add(&entry->tv.head, validated); } -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] drm/tegra: dpaux: Fix always-failing probing of the driver
On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote: > Some of definitions in the code changed the meaning, unfortunately one > place missed the change. > > Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support") > Cc: # v4.8+ > Signed-off-by: Dmitry Osipenko > --- > > I don't have HW to test DPAUX driver, apparently it has been broken for > 2+ years now. There is also a known issue on with the DPAUX driver that > prevents it from probing, that was discussed on the #tegra IRC. Thierry, > please take a closer look at this driver and test it thoroughly, it has > some obvious problems. > > drivers/gpu/drm/tegra/dpaux.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) It's odd that you claim that the driver is always failing probe and at the same time you say that you don't have hardware to test the driver. =) I know for a fact that this driver does not usually fail because it is required on all recent chips (Tegra210 and later) to drive HDMI, which we support on all boards, so it is indeed thoroughly tested. > > diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c > index d84e81ff36ad..ba5681fab73b 100644 > --- a/drivers/gpu/drm/tegra/dpaux.c > +++ b/drivers/gpu/drm/tegra/dpaux.c > @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct platform_device *pdev) >* is no possibility to perform the I2C mode configuration in the >* HDMI path. >*/ > - err = tegra_dpaux_pad_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_I2C); > + err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C); > if (err < 0) > return err; > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which is a good explanation for why the driver performs flawlessly. That said, your change is obviously correct. I've applied it, but since it doesn't actually fix anything, and doesn't change anything from a binary point of view, I've removed the Fixes: and Cc: stable tags. Thanks, Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 0/2] Manage pixel clock & data enable polarities
Version 1: - Initial commit This serie contains all patchsets needed for control the pixel clock & data enable polarities by the display controller driver. Yannick Fertré (2): drm: Add missing flags for pixel clock & data enable drm/stm: ltdc: Solve issue on pixel clock & data enable polarity drivers/gpu/drm/drm_modes.c | 19 ++- drivers/gpu/drm/stm/ltdc.c | 23 +++ include/uapi/drm/drm_mode.h | 6 ++ 3 files changed, 43 insertions(+), 5 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 1/2] drm: Add missing flags for pixel clock & data enable
Add missing flags for pixel clock & data enable polarities. These flags are similar to other synchronization signals (hsync, vsync...). Signed-off-by: Yannick Fertré --- drivers/gpu/drm/drm_modes.c | 19 ++- include/uapi/drm/drm_mode.h | 6 ++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 02db9ac..596f8b3 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -130,7 +130,7 @@ EXPORT_SYMBOL(drm_mode_probed_add); * according to the hdisplay, vdisplay, vrefresh. * It is based from the VESA(TM) Coordinated Video Timing Generator by * Graham Loveridge April 9, 2003 available at - * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls + * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls * * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c. * What I have done is to translate it by using integer calculation. @@ -611,6 +611,15 @@ void drm_display_mode_from_videomode(const struct videomode *vm, dmode->flags |= DRM_MODE_FLAG_DBLSCAN; if (vm->flags & DISPLAY_FLAGS_DOUBLECLK) dmode->flags |= DRM_MODE_FLAG_DBLCLK; + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE) + dmode->flags |= DRM_MODE_FLAG_PPIXCLK; + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) + dmode->flags |= DRM_MODE_FLAG_NPIXCLK; + if (vm->flags & DISPLAY_FLAGS_DE_HIGH) + dmode->flags |= DRM_MODE_FLAG_PDATAEN; + else if (vm->flags & DISPLAY_FLAGS_DE_LOW) + dmode->flags |= DRM_MODE_FLAG_NDE; + drm_mode_set_name(dmode); } EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode); @@ -652,6 +661,14 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode, vm->flags |= DISPLAY_FLAGS_DOUBLESCAN; if (dmode->flags & DRM_MODE_FLAG_DBLCLK) vm->flags |= DISPLAY_FLAGS_DOUBLECLK; + if (dmode->flags & DRM_MODE_FLAG_PPIXDATA) + vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE; + else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA) + vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE; + if (dmode->flags & DRM_MODE_FLAG_PDE) + vm->flags |= DISPLAY_FLAGS_DE_HIGH; + else if (dmode->flags & DRM_MODE_FLAG_NDE) + vm->flags |= DISPLAY_FLAGS_DE_LOW; } EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode); diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index d3e0fe3..b335a17 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -89,6 +89,12 @@ extern "C" { #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14) #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF(8<<14) +/* flags for polarity clock & data enable polarities */ +#define DRM_MODE_FLAG_PPIXDATA (1 << 19) +#define DRM_MODE_FLAG_NPIXDATA (1 << 20) +#define DRM_MODE_FLAG_PDE (1 << 21) +#define DRM_MODE_FLAG_NDE (1 << 22) + /* Picture aspect ratio options */ #define DRM_MODE_PICTURE_ASPECT_NONE 0 #define DRM_MODE_PICTURE_ASPECT_4_31 -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH 2/2] drm: Use new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags
On 24.09.2018 13:48, Thierry Reding wrote: > On Sat, Sep 22, 2018 at 03:15:04PM +0300, Laurent Pinchart wrote: >> The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE flags are deprecated in favour of >> the new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace >> them through the code. >> >> Signed-off-by: Laurent Pinchart >> --- >> drivers/gpu/drm/bridge/dumb-vga-dac.c| 6 +++--- >> drivers/gpu/drm/drm_modes.c | 8 >> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- >> drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +- >> drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 6 +++--- >> drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c| 2 +- >> .../drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 2 +- >> .../gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c | 2 +- >> .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c | 2 +- >> .../gpu/drm/omapdrm/displays/panel-sony-acx565akm.c | 2 +- >> .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 2 +- >> .../gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c | 2 +- >> drivers/gpu/drm/omapdrm/dss/dsi.c| 2 +- >> drivers/gpu/drm/omapdrm/dss/sdi.c| 2 +- >> drivers/gpu/drm/omapdrm/omap_encoder.c | 4 ++-- >> drivers/gpu/drm/panel/panel-arm-versatile.c | 4 ++-- >> drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 4 ++-- >> drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 2 +- >> drivers/gpu/drm/panel/panel-simple.c | 20 >> ++-- >> drivers/gpu/drm/pl111/pl111_display.c| 2 +- >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- >> drivers/gpu/drm/tve200/tve200_display.c | 3 ++- >> include/drm/drm_bridge.h | 8 >> 23 files changed, 47 insertions(+), 46 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c >> index 9b706789a341..7dc14c22f7db 100644 >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev) >> */ >> static const struct drm_bridge_timings default_dac_timings = { >> /* Timing specifications, datasheet page 7 */ >> -.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> +.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, >> .setup_time_ps = 500, >> .hold_time_ps = 1500, >> }; >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings >> default_dac_timings = { >> */ >> static const struct drm_bridge_timings ti_ths8134_dac_timings = { >> /* From timing diagram, datasheet page 9 */ >> -.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> +.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, >> /* From datasheet, page 12 */ >> .setup_time_ps = 3000, >> /* I guess this means latched input */ >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings >> ti_ths8134_dac_timings = { >> */ >> static const struct drm_bridge_timings ti_ths8135_dac_timings = { >> /* From timing diagram, datasheet page 14 */ >> -.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> +.sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, >> /* From datasheet, page 16 */ >> .setup_time_ps = 2000, >> .hold_time_ps = 500, > > Now I'm confused. Aren't you effectively iverting the sampling edges > here? The meaning of the flag has been defined differently for the sampling_edge case, see current comment in include/drm/drm_bridge.h. This is correct. It essentially fixes the flags such that they can be interpreted by the driver with the usual assumption to drive on opposite edge. It is essentially the same as my patch did, but using the new flag: https://patchwork.kernel.org/patch/10589233/ So far, no driver used sampling_edge, so we can change safely at this point. -- Stefan > > That and the fact that everywhere else we only use the driver variants > makes me think that we should just define which way these are supposed > to be used and just have a single set of definitions. > > Also, I think there's no need for these to be always "physically" > correct. This is up to interpretation by the driver, so if a bridge > driver wants to use them as meaning "sampled edge", that's fine. If > display controllers use them to mean "driven edge", that's equally > fine. > > As long as we don't communicate the flags between drivers there should > be no reason for them to be confusing. Just make sure that the comments > in the interfaces clarify from which point of view these flags are to be > interpreted and we should be fine. > > Thierry ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 2/2] drm/stm: ltdc: Solve issue on pixel clock & data enable polarity
Wrong flags used for set the pixel clock & data enable polarities. Add trace for polarities of hsync, vsync, data enabled & pixel clock. Signed-off-by: Yannick Fertré --- drivers/gpu/drm/stm/ltdc.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 808d9fb..f671abc 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -517,7 +517,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) struct videomode vm; u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h; u32 total_width, total_height; - u32 val; + u32 val = 0; drm_display_mode_to_videomode(mode, &vm); @@ -538,7 +538,22 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) total_height = accum_act_h + vm.vfront_porch; /* Configures the HS, VS, DE and PC polarities. Default Active Low */ - val = 0; + if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW) + DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is active low"); + if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH) + DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is active high"); + if (vm.flags & DISPLAY_FLAGS_VSYNC_LOW) + DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active low"); + if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH) + DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active high"); + if (vm.flags & DISPLAY_FLAGS_DE_LOW) + DRM_DEBUG_DRIVER("Data Enable polarity is active low"); + if (vm.flags & DISPLAY_FLAGS_DE_HIGH) + DRM_DEBUG_DRIVER("Data Enable polarity is active high"); + if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) + DRM_DEBUG_DRIVER("Pixel clock polarity is active low"); + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE) + DRM_DEBUG_DRIVER("Pixel clock polarity is active high"); if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH) val |= GCR_HSPOL; @@ -546,10 +561,10 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH) val |= GCR_VSPOL; - if (vm.flags & DISPLAY_FLAGS_DE_HIGH) + if (vm.flags & DISPLAY_FLAGS_DE_LOW) val |= GCR_DEPOL; - if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE) val |= GCR_PCPOL; reg_update_bits(ldev->regs, LTDC_GCR, -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH 2/2] drm: Use new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags
On Mon, Sep 24, 2018 at 01:59:25PM +0200, Stefan Agner wrote: > On 24.09.2018 13:48, Thierry Reding wrote: > > On Sat, Sep 22, 2018 at 03:15:04PM +0300, Laurent Pinchart wrote: > >> The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE flags are deprecated in favour of > >> the new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace > >> them through the code. > >> > >> Signed-off-by: Laurent Pinchart > >> --- > >> drivers/gpu/drm/bridge/dumb-vga-dac.c| 6 +++--- > >> drivers/gpu/drm/drm_modes.c | 8 > >> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- > >> drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +- > >> drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 6 +++--- > >> drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c| 2 +- > >> .../drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 2 +- > >> .../gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c | 2 +- > >> .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c | 2 +- > >> .../gpu/drm/omapdrm/displays/panel-sony-acx565akm.c | 2 +- > >> .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 2 +- > >> .../gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c | 2 +- > >> drivers/gpu/drm/omapdrm/dss/dsi.c| 2 +- > >> drivers/gpu/drm/omapdrm/dss/sdi.c| 2 +- > >> drivers/gpu/drm/omapdrm/omap_encoder.c | 4 ++-- > >> drivers/gpu/drm/panel/panel-arm-versatile.c | 4 ++-- > >> drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 4 ++-- > >> drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 2 +- > >> drivers/gpu/drm/panel/panel-simple.c | 20 > >> ++-- > >> drivers/gpu/drm/pl111/pl111_display.c| 2 +- > >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- > >> drivers/gpu/drm/tve200/tve200_display.c | 3 ++- > >> include/drm/drm_bridge.h | 8 > >> 23 files changed, 47 insertions(+), 46 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c > >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c > >> index 9b706789a341..7dc14c22f7db 100644 > >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c > >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c > >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device > >> *pdev) > >> */ > >> static const struct drm_bridge_timings default_dac_timings = { > >>/* Timing specifications, datasheet page 7 */ > >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, > >> + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > >>.setup_time_ps = 500, > >>.hold_time_ps = 1500, > >> }; > >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings > >> default_dac_timings = { > >> */ > >> static const struct drm_bridge_timings ti_ths8134_dac_timings = { > >>/* From timing diagram, datasheet page 9 */ > >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, > >> + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > >>/* From datasheet, page 12 */ > >>.setup_time_ps = 3000, > >>/* I guess this means latched input */ > >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings > >> ti_ths8134_dac_timings = { > >> */ > >> static const struct drm_bridge_timings ti_ths8135_dac_timings = { > >>/* From timing diagram, datasheet page 14 */ > >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, > >> + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, > >>/* From datasheet, page 16 */ > >>.setup_time_ps = 2000, > >>.hold_time_ps = 500, > > > > Now I'm confused. Aren't you effectively iverting the sampling edges > > here? > > The meaning of the flag has been defined differently for the > sampling_edge case, see current comment in include/drm/drm_bridge.h. > > This is correct. It essentially fixes the flags such that they can be > interpreted by the driver with the usual assumption to drive on opposite > edge. It is essentially the same as my patch did, but using the new > flag: > https://patchwork.kernel.org/patch/10589233/ > > So far, no driver used sampling_edge, so we can change safely at this > point. I was just wondering because the above clearly changes the value in .sampling_edge, but if it isn't currently used it doesn't really matter. One potential problem I see here is that the kerneldoc for sampling_edge says that it should reuse the flags from the DRM connector. Now if the DRM connector specifies these from a drive perspective and the bridge interprets them from a sample perspective, this is obviously going to be a problem. But then, the only places where these are used seems to be in the VGA bridge driver, so I'm assuming that these are from a sampling perspective and should be interpreted that way. So I think that's all that we need to define. If a driver specifies the values in some structure, then they should be interpreted from the driver's perspective. If
Re: [PATCH 02/10] phy: Add configuration interface
Hi! (stripping the mail a bit) On Mon, Sep 24, 2018 at 05:25:26PM +0530, Kishon Vijay Abraham I wrote: > > This integration will prevent us to use some clock rates on the first > > SoC, while the second one would be totally fine with it. > > If there's a clock that is fed to the PHY from the consumer, then the > consumer > driver should model a clock provider and the PHY can get a reference to > it > using clk_get(). Rockchip and Arasan eMMC PHYs has already used > something like > that. > >>> > >>> That would be doable, but no current driver has had this in their > >>> binding. So that would prevent any further rework, and make that whole > >>> series moot. And while I could live without the Allwinner part, the > >>> Cadence one is really needed. > >> > >> We could add a binding and modify the driver to to register a clock > >> provider. > >> That could be included in this series itself. > > > > That wouldn't work for device whose bindings need to remain backward > > compatible. And the Allwinner part at least is in that case. > > Er.. > > > I think we should aim at making it a norm for newer bindings, but we > > still have to support the old ones that cannot be changed. > > There are drivers which support both the old and new bindings. Allwinner could > be in that category. Yes, definitely. However, in order to support the old binding, we'd still need to have a way to give the clock rates to the phy when we don't have that clock provider. So I don't really see how we could remove those fields, even if we start introducing new bindings. > >>> So the timings expressed in the structure are the set of all the ones > >>> currently used in the tree by DSI and CSI drivers. I would consider > >>> that a good proof that it would be useful. > >> > >> The problem I see here is each platform (PHY) will have it's own set of > >> parameters and we have to keep adding members to phy_configure_opts which > >> is > >> not scalable. We should try to find a correlation between generic PHY > >> modes and > >> these parameters (at-least for a subset). > > > > I definitely understand you skepticism towards someone coming in and > > dropping such a big list of obscure parameters :) > > That's part of my concern. The other concern is consumer driver having to know > so much internal details of the PHY. None of the other consumers (PCIe, USB, > etc) had to know so much internals of the PHY. I guess that's true to some extent, but it also feels a bit like a self-realizing prophecy. The phy framework also didn't provide any way to change the phy configuration based on some runtime values up until now, and we have a good example with the MIPI-DSI and MIPI-CSI drivers that given the constructs used in these drivers, if the phy framework had allowed it, they would have used it. Also, speaking of USB, we've had some configuration that was done in some private functions exported by the phy drivers themselves. So there is a use case for such an interface even for other, less configurable, phy types. I even planned for that, since the phy_validate and phy_configure functions are passed an union, in order to make it extensible to other phy types easily. I guess that both the fact that the configuration is simpler for the phy types supported so far, and that the phy and its consumer are very integrated, didn't make it necessary to have such an interface so far. But with MIPI-DPHY, we have both a quite extensive configuration to make, and the phys and their consumers seem to be a bit more loosely integrated. HDMI phys seem to be in pretty much the same case too. > > However, those values are actually the whole list of parameters > > defined by the MIPI-DPHY standard, so I really don't expect drivers to > > need more than that. As you can see, most drivers allow less > > parameters to be configured, but all of them are defined within those > > parameters. So I'm not sure we need to worry about an ever-expanding > > list of parameters: we have a limited set of parameters defined, and > > from an authoritative source, so we can also push back if someone > > wants to add a parameter that is implementation specific. > > Your commit log mentioned there are a few parameters in addition to what is > specified in the MIPI D-PHY spec :-/ > > "The parameters added here are the one defined in the MIPI D-PHY spec, plus > some parameters that were used by a number of PHY drivers currently found > in the linux kernel." I should have phrased that better then, sorry. The only additions that were made were the lanes number (that we agreed to remove), the high speed and low power clock rates, and the video timings. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/
Re: [PATCH v4 5/5] drm: bridge: add support for Cadence MHDP DPI/DP bridge
Hi Damian, Am Montag, 24. September 2018, 13:40:03 CEST schrieb Damian Kos: > > Am Donnerstag, 20. September 2018, 16:54:40 CEST schrieb Damian Kos: > > > From: Quentin Schulz > > > > > > This adds basic support for Cadence MHDP DPI to DP bridge. > > > > > > Basically, it takes a DPI stream as input and output it encoded in DP > > > format. It's missing proper HPD, HDCP and currently supports only SST > > > mode. > > > > > > Changes made in the low level driver (cdn-dp-reg.*): > > > - moved it to from drivers/gpu/drm/rockchip to > > > drivers/gpu/drm/bridge/cdns-mhdp-common.* > > > - functions for sending/receiving commands are now public > > > - added functions for reading registers and link training > > > adjustment > > > > > > Changes made in RK's driver (cdn-dp-core.*): > > > - Moved audio_info and audio_pdev fields from cdn_dp_device to > > > cdns_mhdp_device structure. > > > > > > Signed-off-by: Quentin Schulz > > > Signed-off-by: Damian Kos > > > > [...] > > > > > diff --git a/drivers/gpu/drm/rockchip/Kconfig > > > b/drivers/gpu/drm/rockchip/Kconfig > > > index 0ccc76217ee4..129b0529f3e1 100644 > > > --- a/drivers/gpu/drm/rockchip/Kconfig > > > +++ b/drivers/gpu/drm/rockchip/Kconfig > > > @@ -27,7 +27,9 @@ config ROCKCHIP_ANALOGIX_DP > > > > > > config ROCKCHIP_CDN_DP > > > bool "Rockchip cdn DP" > > > - depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m) > > > + depends on DRM_ROCKCHIP=m > > > > Sorry, I wasn't fast enough in my reply to you mail to catch that before > > your v4, but I don't think this is necessary. > > Instead I do guess, the select below should do the right thing by making > > EXTCON=y if DRM_ROCKCHIP=y. > > > > Somewhat clumsily verified by making EXTCON=m in my defconfig and seeing > > get changed to y upon build, which I guess comes from a different "select" > > in the config. > > > > I've changed it to: > > config ROCKCHIP_CDN_DP > bool "Rockchip cdn DP" > depends on DRM_ROCKCHIP > select EXTCON if DRM_ROCKCHIP=y > select DRM_CDNS_MHDP > help... > > and it seems that there are no issues. At least for me. > > Please let me know if that's OK. nope, just do "select EXTCON" . It works nicely and the above would not select EXTCON at all if the the rockchip-drm driver gets built into the kernel. To be sure I just did a little experiment and added: diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 4ad85c046dcd..64e2b096afd8 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -154,4 +154,11 @@ source "drivers/gpu/drm/bridge/ite/Kconfig" source "drivers/gpu/drm/bridge/synopsys/Kconfig" +config DRM_TMP1 + tristate "option to select" + +config DRM_TMP2 + tristate "option doing the selecting" + select DRM_TMP1 + If I select DRM_TMP2=m as module I end up with CONFIG_DRM_TMP1=m CONFIG_DRM_TMP2=m in the defconfig and wit DRM_TMP2=y it changes to CONFIG_DRM_TMP1=y CONFIG_DRM_TMP2=y so all is well with just doing "select EXTCON" above and will select the correct "m" or "y" stance as needed. Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] drm/tegra: dpaux: Fix always-failing probing of the driver
On 24/09/18 12:59, Thierry Reding wrote: > On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote: >> Some of definitions in the code changed the meaning, unfortunately one >> place missed the change. >> >> Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support") >> Cc: # v4.8+ >> Signed-off-by: Dmitry Osipenko >> --- >> >> I don't have HW to test DPAUX driver, apparently it has been broken for >> 2+ years now. There is also a known issue on with the DPAUX driver that >> prevents it from probing, that was discussed on the #tegra IRC. Thierry, >> please take a closer look at this driver and test it thoroughly, it has >> some obvious problems. >> >> drivers/gpu/drm/tegra/dpaux.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > It's odd that you claim that the driver is always failing probe and at > the same time you say that you don't have hardware to test the driver. > =) > > I know for a fact that this driver does not usually fail because it is > required on all recent chips (Tegra210 and later) to drive HDMI, which > we support on all boards, so it is indeed thoroughly tested. > >> >> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c >> index d84e81ff36ad..ba5681fab73b 100644 >> --- a/drivers/gpu/drm/tegra/dpaux.c >> +++ b/drivers/gpu/drm/tegra/dpaux.c >> @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct platform_device >> *pdev) >> * is no possibility to perform the I2C mode configuration in the >> * HDMI path. >> */ >> -err = tegra_dpaux_pad_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_I2C); >> +err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C); >> if (err < 0) >> return err; >> > > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and > DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which > is a good explanation for why the driver performs flawlessly. > > That said, your change is obviously correct. I've applied it, but since > it doesn't actually fix anything, and doesn't change anything from a > binary point of view, I've removed the Fixes: and Cc: stable tags. Did you change the subject for the patch because as you mentioned it does not seem related to the change? Otherwise for the fix you can have my ... Acked-by: Jon Hunter The dpaux driver has been working fine for me on Tegra210 Smaug when using the pins for I2C and I have not seen any probing problems. Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH 2/2] drm: Use new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags
On 24.09.2018 14:13, Thierry Reding wrote: > On Mon, Sep 24, 2018 at 01:59:25PM +0200, Stefan Agner wrote: >> On 24.09.2018 13:48, Thierry Reding wrote: >> > On Sat, Sep 22, 2018 at 03:15:04PM +0300, Laurent Pinchart wrote: >> >> The DRM_BUS_FLAG_PIXDATA_(POS|NEG)EDGE flags are deprecated in favour of >> >> the new DRM_BUS_FLAG_PIXDATA_(DRIVE|SAMPLE)_(POS|NEG)EDGE flags. Replace >> >> them through the code. >> >> >> >> Signed-off-by: Laurent Pinchart >> >> >> >> --- >> >> drivers/gpu/drm/bridge/dumb-vga-dac.c| 6 +++--- >> >> drivers/gpu/drm/drm_modes.c | 8 >> >> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- >> >> drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +- >> >> drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 6 +++--- >> >> drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c| 2 +- >> >> .../drm/omapdrm/displays/panel-lgphilips-lb035q02.c | 2 +- >> >> .../gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c | 2 +- >> >> .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c | 2 +- >> >> .../gpu/drm/omapdrm/displays/panel-sony-acx565akm.c | 2 +- >> >> .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 2 +- >> >> .../gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c | 2 +- >> >> drivers/gpu/drm/omapdrm/dss/dsi.c| 2 +- >> >> drivers/gpu/drm/omapdrm/dss/sdi.c| 2 +- >> >> drivers/gpu/drm/omapdrm/omap_encoder.c | 4 ++-- >> >> drivers/gpu/drm/panel/panel-arm-versatile.c | 4 ++-- >> >> drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 4 ++-- >> >> drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 2 +- >> >> drivers/gpu/drm/panel/panel-simple.c | 20 >> >> ++-- >> >> drivers/gpu/drm/pl111/pl111_display.c| 2 +- >> >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- >> >> drivers/gpu/drm/tve200/tve200_display.c | 3 ++- >> >> include/drm/drm_bridge.h | 8 >> >> 23 files changed, 47 insertions(+), 46 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c >> >> index 9b706789a341..7dc14c22f7db 100644 >> >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c >> >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device >> >> *pdev) >> >> */ >> >> static const struct drm_bridge_timings default_dac_timings = { >> >> /* Timing specifications, datasheet page 7 */ >> >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> >> + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, >> >> .setup_time_ps = 500, >> >> .hold_time_ps = 1500, >> >> }; >> >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings >> >> default_dac_timings = { >> >> */ >> >> static const struct drm_bridge_timings ti_ths8134_dac_timings = { >> >> /* From timing diagram, datasheet page 9 */ >> >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> >> + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, >> >> /* From datasheet, page 12 */ >> >> .setup_time_ps = 3000, >> >> /* I guess this means latched input */ >> >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings >> >> ti_ths8134_dac_timings = { >> >> */ >> >> static const struct drm_bridge_timings ti_ths8135_dac_timings = { >> >> /* From timing diagram, datasheet page 14 */ >> >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> >> + .sampling_edge = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, >> >> /* From datasheet, page 16 */ >> >> .setup_time_ps = 2000, >> >> .hold_time_ps = 500, >> > >> > Now I'm confused. Aren't you effectively iverting the sampling edges >> > here? >> >> The meaning of the flag has been defined differently for the >> sampling_edge case, see current comment in include/drm/drm_bridge.h. >> >> This is correct. It essentially fixes the flags such that they can be >> interpreted by the driver with the usual assumption to drive on opposite >> edge. It is essentially the same as my patch did, but using the new >> flag: >> https://patchwork.kernel.org/patch/10589233/ >> >> So far, no driver used sampling_edge, so we can change safely at this >> point. > > I was just wondering because the above clearly changes the value in > .sampling_edge, but if it isn't currently used it doesn't really matter. > > One potential problem I see here is that the kerneldoc for sampling_edge > says that it should reuse the flags from the DRM connector. Now if the > DRM connector specifies these from a drive perspective and the bridge > interprets them from a sample perspective, this is obviously going to be > a problem. But then, the only places where these are used seems to be in > the VGA bridge driver, so I'm assuming that these are from a sampling > perspective and should be interpreted that way. > > So I think that's all
[Bug 199139] System freezes (kernel, amdgpu NULL pointer dereference) when video enters powersafe state
https://bugzilla.kernel.org/show_bug.cgi?id=199139 --- Comment #16 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- The logs are sufficient in case to understand what issue you're experiencing. However, I'm surprised that this is occurring with xf86-video-amdgpu. Another user with a similar setup was reporting this problem only occurring with the modesetting driver - but they were using GNOME instead of xfce. -- 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 108030] Does not work discrete graphics CERAR (6370M)
https://bugs.freedesktop.org/show_bug.cgi?id=108030 --- Comment #1 from Alex Deucher --- Please attach your xorg log (if using X) and dmesg output. -- 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 108020] Screen artifacting with 4.18+ kernels on AMD Radeon Vega 64 & 1440p 120hz+ monitors
https://bugs.freedesktop.org/show_bug.cgi?id=108020 --- Comment #2 from Nicholas Kazlauskas --- This is likely fixed. It may be worth testing the amd-staging-drm-next kernel from: https://cgit.freedesktop.org/~agd5f/linux/ or applying the proposed patch from the bug report at: https://bugzilla.kernel.org/show_bug.cgi?id=201067 Please let me know if you're seeing an issue with either of these. -- 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 2/2] MAINTAINERS: Move mxsfb drm driver to drm-misc tree
On 19.09.2018 22:39, Sean Paul wrote: > From: Sean Paul > > Another "small driver" moving into drm-misc. Stefan has also offered to > co-maintain it. > > Cc: Marek Vasut > Cc: Stefan Agner > Cc: David Airlie > Cc: Gustavo Padovan > Cc: Maarten Lankhorst > Signed-off-by: Sean Paul Marek, I guess Sean needs your ack here. Are you fine with this? -- Stefan > --- > MAINTAINERS | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index ba1dbb8735b8..7184d53dcbd8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9891,9 +9891,12 @@ F: drivers/media/tuners/mxl5007t.* > > MXSFB DRM DRIVER > M: Marek Vasut > +M: Stefan Agner > +L: dri-devel@lists.freedesktop.org > S: Supported > F: drivers/gpu/drm/mxsfb/ > F: Documentation/devicetree/bindings/display/mxsfb.txt > +T: git git://anongit.freedesktop.org/drm/drm-misc > > MYRICOM MYRI-10G 10GbE DRIVER (MYRI10GE) > M: Chris Lee ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] drm/tegra: dpaux: Fix always-failing probing of the driver
On Mon, Sep 24, 2018 at 01:32:41PM +0100, Jon Hunter wrote: > > On 24/09/18 12:59, Thierry Reding wrote: > > On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote: > >> Some of definitions in the code changed the meaning, unfortunately one > >> place missed the change. > >> > >> Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support") > >> Cc: # v4.8+ > >> Signed-off-by: Dmitry Osipenko > >> --- > >> > >> I don't have HW to test DPAUX driver, apparently it has been broken for > >> 2+ years now. There is also a known issue on with the DPAUX driver that > >> prevents it from probing, that was discussed on the #tegra IRC. Thierry, > >> please take a closer look at this driver and test it thoroughly, it has > >> some obvious problems. > >> > >> drivers/gpu/drm/tegra/dpaux.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > It's odd that you claim that the driver is always failing probe and at > > the same time you say that you don't have hardware to test the driver. > > =) > > > > I know for a fact that this driver does not usually fail because it is > > required on all recent chips (Tegra210 and later) to drive HDMI, which > > we support on all boards, so it is indeed thoroughly tested. > > > >> > >> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c > >> index d84e81ff36ad..ba5681fab73b 100644 > >> --- a/drivers/gpu/drm/tegra/dpaux.c > >> +++ b/drivers/gpu/drm/tegra/dpaux.c > >> @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct platform_device > >> *pdev) > >> * is no possibility to perform the I2C mode configuration in the > >> * HDMI path. > >> */ > >> - err = tegra_dpaux_pad_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_I2C); > >> + err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C); > >>if (err < 0) > >>return err; > >> > > > > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and > > DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which > > is a good explanation for why the driver performs flawlessly. > > > > That said, your change is obviously correct. I've applied it, but since > > it doesn't actually fix anything, and doesn't change anything from a > > binary point of view, I've removed the Fixes: and Cc: stable tags. > > Did you change the subject for the patch because as you mentioned it > does not seem related to the change? Yes, I did. > Otherwise for the fix you can have my ... > > Acked-by: Jon Hunter Added, thanks. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108033] Kernel crash on videogames and high resolution video
https://bugs.freedesktop.org/show_bug.cgi?id=108033 --- Comment #1 from Alex Deucher --- Other than the warnings in the log, is there any actual problem? -- 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 v1] drm/tegra: dpaux: Fix always-failing probing of the driver
On Mon, Sep 24, 2018 at 03:40:35PM +0300, Dmitry Osipenko wrote: > On 9/24/18 3:32 PM, Jon Hunter wrote: > > > > On 24/09/18 12:59, Thierry Reding wrote: > > > On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote: > > > > Some of definitions in the code changed the meaning, unfortunately one > > > > place missed the change. > > > > > > > > Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support") > > > > Cc: # v4.8+ > > > > Signed-off-by: Dmitry Osipenko > > > > --- > > > > > > > > I don't have HW to test DPAUX driver, apparently it has been broken for > > > > 2+ years now. There is also a known issue on with the DPAUX driver that > > > > prevents it from probing, that was discussed on the #tegra IRC. Thierry, > > > > please take a closer look at this driver and test it thoroughly, it has > > > > some obvious problems. > > > > > > > > drivers/gpu/drm/tegra/dpaux.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > It's odd that you claim that the driver is always failing probe and at > > > the same time you say that you don't have hardware to test the driver. > > > =) > > > > > > I know for a fact that this driver does not usually fail because it is > > > required on all recent chips (Tegra210 and later) to drive HDMI, which > > > we support on all boards, so it is indeed thoroughly tested. > > > > > > > > > > > diff --git a/drivers/gpu/drm/tegra/dpaux.c > > > > b/drivers/gpu/drm/tegra/dpaux.c > > > > index d84e81ff36ad..ba5681fab73b 100644 > > > > --- a/drivers/gpu/drm/tegra/dpaux.c > > > > +++ b/drivers/gpu/drm/tegra/dpaux.c > > > > @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct platform_device > > > > *pdev) > > > > * is no possibility to perform the I2C mode configuration in > > > > the > > > > * HDMI path. > > > > */ > > > > - err = tegra_dpaux_pad_config(dpaux, > > > > DPAUX_HYBRID_PADCTL_MODE_I2C); > > > > + err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C); > > > > if (err < 0) > > > > return err; > > > > > > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and > > > DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which > > > is a good explanation for why the driver performs flawlessly. > > > > > > That said, your change is obviously correct. I've applied it, but since > > > it doesn't actually fix anything, and doesn't change anything from a > > > binary point of view, I've removed the Fixes: and Cc: stable tags. > > > > Did you change the subject for the patch because as you mentioned it > > does not seem related to the change? > > > > Otherwise for the fix you can have my ... > > > > Acked-by: Jon Hunter > > > > The dpaux driver has been working fine for me on Tegra210 Smaug when > > using the pins for I2C and I have not seen any probing problems. > > Guy with nickname "vlado" said on the IRC that the panel stopped to work on > T124 chromebook since 4.16 kernel, reverting commit [0] helps. > > [0] > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/drm_dp_helper.c?id=17ab7806de0c10d27cdbda8ef57ca680bdd24315 I'm pretty sure that's something that we fixed, but I'll have to check. Is this patch reported to help with that issue, or where's the connection? Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108033] Kernel crash on videogames and high resolution video
https://bugs.freedesktop.org/show_bug.cgi?id=108033 --- Comment #2 from lae...@pm.me --- yea, kernel crashing and not responding after that, i have to reboot my PC. i'm not sure how can i retrieve more information to make this clearer. -- 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 v1] drm/tegra: dpaux: Fix always-failing probing of the driver
On Mon, Sep 24, 2018 at 03:36:29PM +0200, Thierry Reding wrote: > On Mon, Sep 24, 2018 at 03:40:35PM +0300, Dmitry Osipenko wrote: > > On 9/24/18 3:32 PM, Jon Hunter wrote: > > > > > > On 24/09/18 12:59, Thierry Reding wrote: > > > > On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote: > > > > > Some of definitions in the code changed the meaning, unfortunately one > > > > > place missed the change. > > > > > > > > > > Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support") > > > > > Cc: # v4.8+ > > > > > Signed-off-by: Dmitry Osipenko > > > > > --- > > > > > > > > > > I don't have HW to test DPAUX driver, apparently it has been broken > > > > > for > > > > > 2+ years now. There is also a known issue on with the DPAUX driver > > > > > that > > > > > prevents it from probing, that was discussed on the #tegra IRC. > > > > > Thierry, > > > > > please take a closer look at this driver and test it thoroughly, it > > > > > has > > > > > some obvious problems. > > > > > > > > > > drivers/gpu/drm/tegra/dpaux.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > It's odd that you claim that the driver is always failing probe and at > > > > the same time you say that you don't have hardware to test the driver. > > > > =) > > > > > > > > I know for a fact that this driver does not usually fail because it is > > > > required on all recent chips (Tegra210 and later) to drive HDMI, which > > > > we support on all boards, so it is indeed thoroughly tested. > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/tegra/dpaux.c > > > > > b/drivers/gpu/drm/tegra/dpaux.c > > > > > index d84e81ff36ad..ba5681fab73b 100644 > > > > > --- a/drivers/gpu/drm/tegra/dpaux.c > > > > > +++ b/drivers/gpu/drm/tegra/dpaux.c > > > > > @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct > > > > > platform_device *pdev) > > > > >* is no possibility to perform the I2C mode configuration in > > > > > the > > > > >* HDMI path. > > > > >*/ > > > > > - err = tegra_dpaux_pad_config(dpaux, > > > > > DPAUX_HYBRID_PADCTL_MODE_I2C); > > > > > + err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C); > > > > > if (err < 0) > > > > > return err; > > > > > > > > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and > > > > DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which > > > > is a good explanation for why the driver performs flawlessly. > > > > > > > > That said, your change is obviously correct. I've applied it, but since > > > > it doesn't actually fix anything, and doesn't change anything from a > > > > binary point of view, I've removed the Fixes: and Cc: stable tags. > > > > > > Did you change the subject for the patch because as you mentioned it > > > does not seem related to the change? > > > > > > Otherwise for the fix you can have my ... > > > > > > Acked-by: Jon Hunter > > > > > > The dpaux driver has been working fine for me on Tegra210 Smaug when > > > using the pins for I2C and I have not seen any probing problems. > > > > Guy with nickname "vlado" said on the IRC that the panel stopped to work on > > T124 chromebook since 4.16 kernel, reverting commit [0] helps. > > > > [0] > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/drm_dp_helper.c?id=17ab7806de0c10d27cdbda8ef57ca680bdd24315 > > I'm pretty sure that's something that we fixed, but I'll have to check. Looks like we didn't fix that after all. Going through the patchwork log linked to from that commit message I thought there wasn't actually an issue with the code, but it turns out that I was wrong. The problem is that for HDMI we don't use DPAUX in I2C mode, so we don't need to look up the corresponding I2C controller and we just need the DPAUX driver to program the pins into I2C mode, then use a real I2C controller for communication. What's special about Nyan is that it uses eDP and for eDP we use the DPAUX for access to the DPCD *and* for DDC. eDP panels will look up the phandle to the I2C controller used for EDID (DDC) via the ddc-i2c-bus property. In order for that to work, we need to register the DPAUX I2C controller with its OF node set to that of the DPAUX device. Technically I don't think we need to query the EDID because we always have at least one hard-coded mode in panel-simple. However, if the panel has a slightly different mode from that, it'd still be good if the driver used that instead of the built-in mode. So I think we need to either bring back that patch or find some alternative way to make the panel aware of the DDC bus that it needs to talk to. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/7] drm/ttm: allow reserve more than one shared slot
The shortlog should say "allow reserving more than one shared fence slot", same as patch 2. On 2018-09-24 1:58 p.m., Christian König wrote: > Let's support simultaneous submissions to multiple engines. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/ttm/ttm_execbuf_util.c | 6 -- > include/drm/ttm/ttm_execbuf_util.h | 2 +- > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c > b/drivers/gpu/drm/ttm/ttm_execbuf_util.c > index e493edb0d3e7..9b842884530a 100644 > --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c > +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c > @@ -129,7 +129,8 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, > if (!entry->shared) > continue; > > - ret = reservation_object_reserve_shared(bo->resv, 1); > + ret = reservation_object_reserve_shared(bo->resv, > + entry->shared); > if (!ret) > continue; > } > @@ -151,7 +152,8 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, > } > > if (!ret && entry->shared) > - ret = reservation_object_reserve_shared(bo->resv, 1); > + ret = reservation_object_reserve_shared(bo->resv, > + entry->shared); > > if (unlikely(ret != 0)) { > if (ret == -EINTR) > diff --git a/include/drm/ttm/ttm_execbuf_util.h > b/include/drm/ttm/ttm_execbuf_util.h > index b0fdd1980034..3b16dda9bd08 100644 > --- a/include/drm/ttm/ttm_execbuf_util.h > +++ b/include/drm/ttm/ttm_execbuf_util.h > @@ -46,7 +46,7 @@ > struct ttm_validate_buffer { > struct list_head head; > struct ttm_buffer_object *bo; > - bool shared; > + unsigned int shared; > }; > > /** > It would be better to change the name of the field to something more descriptive, e.g. "num_shared_fence_contexts", and fix up all users accordingly in this patch (which will get rid of one hunk in patch 4). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] drm/amdgpu: always reserve two slots for the VM
On 2018-09-24 1:58 p.m., Christian König wrote: > And drop the now superflous extra reservations. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +- > 2 files changed, 5 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 1a2ea2e931e3..cda8cf90777c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -962,10 +962,6 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser > *p) > if (r) > return r; > > - r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv, 1); > - if (r) > - return r; > - > p->job->vm_pd_addr = amdgpu_gmc_pd_addr(vm->root.base.bo); > > if (amdgpu_vm_debug) { > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index bdce05183edb..353367382874 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -616,7 +616,7 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, > { > entry->priority = 0; > entry->tv.bo = &vm->root.base.bo->tbo; > - entry->tv.shared = true; > + entry->tv.shared = 2; Where assigning values > 1 to this field, it would be good to have a comment explaining what the multiple shared fence slots correspond to. Same for patch 6. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] drm/tegra: dpaux: Fix always-failing probing of the driver
On Mon, Sep 24, 2018 at 04:56:28PM +0300, Dmitry Osipenko wrote: > On 9/24/18 4:36 PM, Thierry Reding wrote: > > On Mon, Sep 24, 2018 at 03:40:35PM +0300, Dmitry Osipenko wrote: > > > On 9/24/18 3:32 PM, Jon Hunter wrote: > > > > > > > > On 24/09/18 12:59, Thierry Reding wrote: > > > > > On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote: > > > > > > Some of definitions in the code changed the meaning, unfortunately > > > > > > one > > > > > > place missed the change. > > > > > > > > > > > > Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support") > > > > > > Cc: # v4.8+ > > > > > > Signed-off-by: Dmitry Osipenko > > > > > > --- > > > > > > > > > > > > I don't have HW to test DPAUX driver, apparently it has been broken > > > > > > for > > > > > > 2+ years now. There is also a known issue on with the DPAUX driver > > > > > > that > > > > > > prevents it from probing, that was discussed on the #tegra IRC. > > > > > > Thierry, > > > > > > please take a closer look at this driver and test it thoroughly, it > > > > > > has > > > > > > some obvious problems. > > > > > > > > > > > >drivers/gpu/drm/tegra/dpaux.c | 2 +- > > > > > >1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > It's odd that you claim that the driver is always failing probe and at > > > > > the same time you say that you don't have hardware to test the driver. > > > > > =) > > > > > > > > > > I know for a fact that this driver does not usually fail because it is > > > > > required on all recent chips (Tegra210 and later) to drive HDMI, which > > > > > we support on all boards, so it is indeed thoroughly tested. > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/tegra/dpaux.c > > > > > > b/drivers/gpu/drm/tegra/dpaux.c > > > > > > index d84e81ff36ad..ba5681fab73b 100644 > > > > > > --- a/drivers/gpu/drm/tegra/dpaux.c > > > > > > +++ b/drivers/gpu/drm/tegra/dpaux.c > > > > > > @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct > > > > > > platform_device *pdev) > > > > > > * is no possibility to perform the I2C mode configuration in > > > > > > the > > > > > > * HDMI path. > > > > > > */ > > > > > > - err = tegra_dpaux_pad_config(dpaux, > > > > > > DPAUX_HYBRID_PADCTL_MODE_I2C); > > > > > > + err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C); > > > > > > if (err < 0) > > > > > > return err; > > > > > > > > > > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C > > > > > and > > > > > DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, > > > > > which > > > > > is a good explanation for why the driver performs flawlessly. > > > > > > > > > > That said, your change is obviously correct. I've applied it, but > > > > > since > > > > > it doesn't actually fix anything, and doesn't change anything from a > > > > > binary point of view, I've removed the Fixes: and Cc: stable tags. > > > > > > > > Did you change the subject for the patch because as you mentioned it > > > > does not seem related to the change? > > > > > > > > Otherwise for the fix you can have my ... > > > > > > > > Acked-by: Jon Hunter > > > > > > > > The dpaux driver has been working fine for me on Tegra210 Smaug when > > > > using the pins for I2C and I have not seen any probing problems. > > > > > > Guy with nickname "vlado" said on the IRC that the panel stopped to work > > > on > > > T124 chromebook since 4.16 kernel, reverting commit [0] helps. > > > > > > [0] > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/drm_dp_helper.c?id=17ab7806de0c10d27cdbda8ef57ca680bdd24315 > > > > I'm pretty sure that's something that we fixed, but I'll have to check. > > > > Is this patch reported to help with that issue, or where's the > > connection? > > Reverting of the patch was reported to help with the problem. Check the IRC > history, you can also just ask the original reporter for more details on > #tegra. IIUC, the DPAUX driver probing is getting deferred because regulator > isn't ready during of the first invocation and then for some reason driver > is never getting re-probed. My IRC logs are a bit spotty because I had some intermittent failure on the server that my client was running on. But I think you're referring to the drm_dp_helper.c patch. I was just getting confused because you referred to it from this thread, and I don't see the connection between the two. Okay, so my other reply has all the background on why this is failing. It would also explain why the driver "always" fails. panel-simple fails to find the I2C adapter for the ddc-i2c-bus property, so it reports -EPROBE_DEFER, which is then propagated to Tegra DRM. So let me resume the discussion on the lists about the offending revert and see if we can get that fixed. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.
[Bug 108037] Turning monitors off and on again makes the kernel panic and system freeze
https://bugs.freedesktop.org/show_bug.cgi?id=108037 --- Comment #2 from Alex Deucher --- When you say turn them off/on, do you mean via software (e.g., dpms) or via the switch on the monitor? -- 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 108020] Screen artifacting with 4.18+ kernels on AMD Radeon Vega 64 & 1440p 120hz+ monitors
https://bugs.freedesktop.org/show_bug.cgi?id=108020 --- Comment #3 from Arek Tumas --- Created attachment 141710 --> https://bugs.freedesktop.org/attachment.cgi?id=141710&action=edit dmesg -- 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 108020] Screen artifacting with 4.18+ kernels on AMD Radeon Vega 64 & 1440p 120hz+ monitors
https://bugs.freedesktop.org/show_bug.cgi?id=108020 --- Comment #4 from Arek Tumas --- Yes it is probably the same bug. It's a right-most vertical interlaced line. I'm currently experiencing this bug in the newest ubuntu mainline kernel (4.19-rc5). [It's not an issue on 4.18.8 kernel] I'm relatively inexperienced, Applying kernel patches seems a little difficult for me. I only know how to install mainline kernels from ukuu or .deb files. I can try if someone links me clear instructions. Thank You for Your time and help. I hope this will be fixed in future Linux kernel versions. -- 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 v2] drm/arm/malidp: Validate rotations for compressed/uncompressed framebuffers for each layer
Hi Liviu, On Fri, Sep 21, 2018 at 03:33:53PM +0100, Liviu Dudau wrote: > From: Ayan Kumar Halder > > Add support for compressed framebuffers that are described using > the framebuffer's modifier field. Mali DP uses the rotation memory for > the decompressor of the format, so we need to check for space when > the modifiers are present. > > Signed-off-by: Ayan Kumar Halder > Reviewed-by: Brian Starkey > [re-worded commit, rebased, cleaned up duplicated checks for > RGB888 and BGR888 and removed additional parameter for > rotmem_required function hook] > Signed-off-by: Liviu Dudau > --- > drivers/gpu/drm/arm/malidp_crtc.c | 28 - > drivers/gpu/drm/arm/malidp_hw.c | 38 - > drivers/gpu/drm/arm/malidp_hw.h | 7 ++ > drivers/gpu/drm/arm/malidp_planes.c | 19 +++ > 4 files changed, 53 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/arm/malidp_crtc.c > b/drivers/gpu/drm/arm/malidp_crtc.c > index ef44202fb43f8..e1b72782848c3 100644 > --- a/drivers/gpu/drm/arm/malidp_crtc.c > +++ b/drivers/gpu/drm/arm/malidp_crtc.c > @@ -348,19 +348,20 @@ static int malidp_crtc_atomic_check(struct drm_crtc > *crtc, > > /* >* check if there is enough rotation memory available for planes > - * that need 90?? and 270?? rotation. Each plane has set its required > - * memory size in the ->plane_check() callback, here we only make > - * sure that the sums are less that the total usable memory. > + * that need 90?? and 270?? rotion or planes that are compressed. > + * Each plane has set its required memory size in the ->plane_check() > + * callback, here we only make sure that the sums are less that the > + * total usable memory. >* >* The rotation memory allocation algorithm (for each plane): > - * a. If no more rotated planes exist, all remaining rotate > - * memory in the bank is available for use by the plane. > - * b. If other rotated planes exist, and plane's layer ID is > - * DE_VIDEO1, it can use all the memory from first bank if > - * secondary rotation memory bank is available, otherwise it can > + * a. If no more rotated or compressed planes exist, all remaining > + * rotate memory in the bank is available for use by the plane. > + * b. If other rotated or compressed planes exist, and plane's > + * layer ID is DE_VIDEO1, it can use all the memory from first bank > + * if secondary rotation memory bank is available, otherwise it can >* use up to half the bank's memory. > - * c. If other rotated planes exist, and plane's layer ID is not > - * DE_VIDEO1, it can use half of the available memory > + * c. If other rotated or compressed planes exist, and plane's layer ID > + * is not DE_VIDEO1, it can use half of the available memory. >* >* Note: this algorithm assumes that the order in which the planes are >* checked always has DE_VIDEO1 plane first in the list if it is > @@ -372,7 +373,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, > > /* first count the number of rotated planes */ > drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) { > - if (pstate->rotation & MALIDP_ROTATED_MASK) > + struct drm_framebuffer *fb = pstate->fb; > + > + if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier) > rotated_planes++; > } > > @@ -388,8 +391,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, > drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) { > struct malidp_plane *mp = to_malidp_plane(plane); > struct malidp_plane_state *ms = to_malidp_plane_state(pstate); > + struct drm_framebuffer *fb = pstate->fb; > > - if (pstate->rotation & MALIDP_ROTATED_MASK) { > + if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier) { > /* process current plane */ > rotated_planes--; > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c > index b33000634a4ee..5549092e6c36a 100644 > --- a/drivers/gpu/drm/arm/malidp_hw.c > +++ b/drivers/gpu/drm/arm/malidp_hw.c > @@ -85,41 +85,43 @@ static const struct malidp_format_id > malidp550_de_formats[] = { > > static const struct malidp_layer malidp500_layers[] = { > /* id, base address, fb pointer address base, stride offset, > - yuv2rgb matrix offset, mmu control register offset */ > + yuv2rgb matrix offset, mmu control register offset, > rotation_features */ > { DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE, > - MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0 }, > + MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, 0, ROTATE_ANY }, > { DE_GRAPHICS1,
[Bug 108030] Does not work discrete graphics CERAR (6370M)
https://bugs.freedesktop.org/show_bug.cgi?id=108030 --- Comment #2 from Илья Индиго --- Created attachment 141711 --> https://bugs.freedesktop.org/attachment.cgi?id=141711&action=edit dmesg -- 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 108030] Does not work discrete graphics CERAR (6370M)
https://bugs.freedesktop.org/show_bug.cgi?id=108030 --- Comment #3 from Илья Индиго --- Created attachment 141712 --> https://bugs.freedesktop.org/attachment.cgi?id=141712&action=edit xorg.log -- 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] drm/fb_helper: Allow leaking fbdev smem_start
On Mon, Sep 24, 2018 at 10:44:05AM +0200, Neil Armstrong wrote: > Hi Maxime, Daniel, > > On 22/09/2018 10:56, Daniel Vetter wrote: > > On Fri, Sep 21, 2018 at 04:07:40PM +0200, Maxime Ripard wrote: > >> Hi, > >> > >> On Thu, Sep 20, 2018 at 03:04:12PM +0200, Neil Armstrong wrote: > >>> Since "drm/fb: Stop leaking physical address", the default behaviour of > >>> the DRM fbdev emulation is to set the smem_base to 0 and pass the new > >>> FBINFO_HIDE_SMEM_START flag. > >>> > >>> The main reason is to avoid leaking physical addresse to user-space, and > >>> it follows a general move over the kernel code to avoid user-space to > >>> manipulate physical addresses and then use some other mechanisms like > >>> dma-buf to transfer physical buffer handles over multiple subsystems. > >>> > >>> But, a lot of devices depends on closed sources binaries to enable > >>> OpenGL hardware acceleration that uses this smem_start value to > >>> pass physical addresses to out-of-tree modules in order to render > >>> into these physical adresses. These should use dma-buf buffers allocated > >>> from the DRM display device instead and stop relying on fbdev > >>> overallocation > >>> to gather DMA memory (some HW vendors delivers GBM and Wayland capable > >>> binaries, but older unsupported devices won't have these new binaries > >>> and are doomed until an Open Source solution like Lima finalizes). > >>> > >>> Since these devices heavily depends on this kind of software and because > >>> the smem_start population was available for years, it's a breakage to > >>> stop leaking smem_start without any alternative solutions. > >>> > >>> This patch adds a Kconfig depending on the EXPERT config and an unsafe > >>> kernel module parameter tainting the kernel when enabled. > >>> > >>> A clear comment and Kconfig help text was added to clarify why and when > >>> this patch should be reverted, but in the meantime it's a necessary > >>> feature to keep. > >>> > >>> Cc: Dave Airlie > >>> Cc: Daniel Vetter > >>> Cc: Bartlomiej Zolnierkiewicz > >>> Cc: Noralf Trønnes > >>> Cc: Maxime Ripard > >>> Cc: Eric Anholt > >>> Cc: Lucas Stach > >>> Cc: Rob Clark > >>> Cc: Ben Skeggs > >>> Cc: Christian König > >>> Signed-off-by: Neil Armstrong > >>> --- > >>> drivers/gpu/drm/Kconfig | 15 +++ > >>> drivers/gpu/drm/drm_fb_helper.c | 33 +++-- > >>> 2 files changed, 46 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > >>> index 8871b3f..b07c298 100644 > >>> --- a/drivers/gpu/drm/Kconfig > >>> +++ b/drivers/gpu/drm/Kconfig > >>> @@ -110,6 +110,21 @@ config DRM_FBDEV_OVERALLOC > >>> is 100. Typical values for double buffering will be 200, > >>> triple buffering 300. > >>> > >>> +config DRM_FBDEV_LEAK_PHYS_SMEM > >>> + bool "Shamelessly allow leaking of fbdev physical address (DANGEROUS)" > >>> + depends on DRM_FBDEV_EMULATION && EXPERT > >>> + default n > >>> + help > >>> + In order to keep user-space compatibility, we want in certain > >>> + use-cases to keep leaking the fbdev physical address to the > >>> + user-space program handling the fbdev buffer. > >>> + This option is a shame and should be removed as soon as possible > >>> + and be considered as a broken and legacy behaviour from a modern > >>> + fbdev device driver. > > > > s/shame/not supported by upstream developers/ > > Ack > > > > > And maybe add "Please send any bug reports when using this to your > > proprietary software vendor that requires this." > > It's (somehow) on the following line : > > >>> + If in doubt, say "N" or spread the word to your closed source > >>> + library vendor. > > I will make it even scarier ! > > > > > I'd also be in favour of just calling this the ARM Mali blob on > > $affected_devices. Makes it easier for people to find the magic knob > > they're looking for, if they're unlucky enough to have such a soc. > > I'm not sure it's only for Mali... Anyway I'll add "Mali" to help > people findind it. > > > > >>> + > >>> + If in doubt, say "N" or spread the word to your closed source > >>> + library vendor. > >>> + > >>> config DRM_LOAD_EDID_FIRMWARE > >>> bool "Allow to specify an EDID data set instead of probing for it" > >>> depends on DRM > >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c > >>> b/drivers/gpu/drm/drm_fb_helper.c > >>> index bf2190c..a354944 100644 > >>> --- a/drivers/gpu/drm/drm_fb_helper.c > >>> +++ b/drivers/gpu/drm/drm_fb_helper.c > >>> @@ -56,6 +56,25 @@ MODULE_PARM_DESC(drm_fbdev_overalloc, > >>>"Overallocation of the fbdev buffer (%) [default=" > >>>__MODULE_STRING(CONFIG_DRM_FBDEV_OVERALLOC) "]"); > >>> > >>> +/* > >>> + * In order to keep user-space compatibility, we want in certain > >>> use-cases > >>> + * to keep leaking the fbdev physical address to the user-space program > >>> + * handling the fbdev buffer. > >>> + * This is a bad habit essentially
[Bug 108030] Does not work discrete graphics CERAR (6370M)
https://bugs.freedesktop.org/show_bug.cgi?id=108030 --- Comment #4 from Илья Индиго --- Created attachment 141713 --> https://bugs.freedesktop.org/attachment.cgi?id=141713&action=edit xsessions-errors -- 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: [RFC 1/3] drm/driver: Add defaults for .gem_prime_export/import callbacks
On Sat, Sep 22, 2018 at 05:58:30PM +0200, Noralf Trønnes wrote: > > Den 22.09.2018 12.10, skrev Daniel Vetter: > > On Fri, Sep 21, 2018 at 06:42:28PM +0200, Noralf Trønnes wrote: > > > The majority of drivers use drm_gem_prime_export() and > > > drm_gem_prime_import() for these callbacks so let's make them the > > > default. > > > > > > Signed-off-by: Noralf Trønnes > > > --- > > > Documentation/gpu/todo.rst | 7 +++ > > > drivers/gpu/drm/drm_prime.c | 10 -- > > > include/drm/drm_drv.h | 4 > > > 3 files changed, 19 insertions(+), 2 deletions(-) > > > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > > index 77c2b3c25565..39748e22dea8 100644 > > > --- a/Documentation/gpu/todo.rst > > > +++ b/Documentation/gpu/todo.rst > > > @@ -234,6 +234,13 @@ efficient. > > > Contact: Daniel Vetter > > > +Defaults for .gem_prime_import and export > > > +- > > > + > > > +Most drivers don't need to set drm_driver->gem_prime_import and > > > +->gem_prime_export now that drm_gem_prime_import() and > > > drm_gem_prime_export() > > > +are the default. > > > + > > > Core refactorings > > > = > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > > index 3f0205fc0a1a..6721571749ff 100644 > > > --- a/drivers/gpu/drm/drm_prime.c > > > +++ b/drivers/gpu/drm/drm_prime.c > > > @@ -559,7 +559,10 @@ static struct dma_buf > > > *export_and_register_object(struct drm_device *dev, > > > return dmabuf; > > > } > > > - dmabuf = dev->driver->gem_prime_export(dev, obj, flags); > > > + if (dev->driver->gem_prime_export) > > > + dmabuf = dev->driver->gem_prime_export(dev, obj, flags); > > > + else > > > + dmabuf = drm_gem_prime_export(dev, obj, flags); > > Hm, this one here operates on a drm_gem_object (the dev parameter is kinda > > redundant). I think this would look neat in the callback structure you're > > adding in the next patch. There's also a bunch of gem drivers overwriting > > this one, so would make your callbacks structure more generally useful. > > I'm not following you here. > What I do is to add a default for the 29 drivers that have this set to > drm_gem_prime_export(). For the 8 drivers that have their own thing, I > have the funcs->export callback that they can use. > > Combined with the next patch it looks like this: > > if (dev->driver->gem_prime_export) > dmabuf = dev->driver->gem_prime_export(dev, obj, flags); > else if (obj->funcs && obj->funcs->export) > dmabuf = obj->funcs->export(obj, flags); > else > dmabuf = drm_gem_prime_export(dev, obj, flags); Oh, I missed that you're adding the funcs->export thing already, that's what I wanted to suggested. -Daniel > > Noralf. > > > Otherwise lgtm. > > -Daniel > > > > > if (IS_ERR(dmabuf)) { > > > /* normally the created dma-buf takes ownership of the > > > ref, > > >* but if that fails then drop the ref > > > @@ -792,7 +795,10 @@ int drm_gem_prime_fd_to_handle(struct drm_device > > > *dev, > > > /* never seen this one, need to import */ > > > mutex_lock(&dev->object_name_lock); > > > - obj = dev->driver->gem_prime_import(dev, dma_buf); > > > + if (dev->driver->gem_prime_import) > > > + obj = dev->driver->gem_prime_import(dev, dma_buf); > > > + else > > > + obj = drm_gem_prime_import(dev, dma_buf); > > > if (IS_ERR(obj)) { > > > ret = PTR_ERR(obj); > > > goto out_unlock; > > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > > > index 8830e3de3a86..1162145f7f68 100644 > > > --- a/include/drm/drm_drv.h > > > +++ b/include/drm/drm_drv.h > > > @@ -471,6 +471,8 @@ struct drm_driver { > > >* @gem_prime_export: > > >* > > >* export GEM -> dmabuf > > > + * > > > + * This defaults to drm_gem_prime_export() if not set. > > >*/ > > > struct dma_buf * (*gem_prime_export)(struct drm_device *dev, > > > struct drm_gem_object *obj, int flags); > > > @@ -478,6 +480,8 @@ struct drm_driver { > > >* @gem_prime_import: > > >* > > >* import dmabuf -> GEM > > > + * > > > + * This defaults to drm_gem_prime_import() if not set. > > >*/ > > > struct drm_gem_object * (*gem_prime_import)(struct drm_device > > > *dev, > > > struct dma_buf *dma_buf); > > > -- > > > 2.15.1 > > > > > > ___ > > > 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 htt
Re: [RFC 2/3] drm/gem: Add drm_gem_object_funcs
On Sat, Sep 22, 2018 at 06:01:55PM +0200, Noralf Trønnes wrote: > > Den 22.09.2018 12.41, skrev Daniel Vetter: > > On Fri, Sep 21, 2018 at 06:42:29PM +0200, Noralf Trønnes wrote: > > > This adds an optional function table on GEM objects. > > > The main benefit is for drivers that support more than one type of > > > memory (shmem,vram,cma) for their buffers depending on the hardware it > > > runs on. With the callbacks attached to the GEM object itself, it is > > > easier to have core helpers for the the various buffer types. The driver > > > only has to make the decision about buffer type on GEM object creation > > > and all other callbacks can be handled by the chosen helper. > > > > > > drm_driver->gem_prime_res_obj has not been added since there's a todo to > > > put a reservation_object into drm_gem_object. > > > > > > The individual drm_driver callbacks take priority over the GEM attached > > > callbacks. > > I'd do this the other way round, make the new gem callbacks the clearly > > favoured option. > > The reason I did it like this is that I thought it would be easier to > convert the CMA helper this way. I've taken a closer look at this, and > the solution is to convert the drivers that override the defaults first, > and then convert the helper as a whole when that's done. > > I'll flip it around. If it's easier for conversion, then keep as-is (but probably with a plan to change the defaults later on). Up to you. > > > Signed-off-by: Noralf Trønnes > > > --- > > > drivers/gpu/drm/drm_client.c| 12 ++-- > > > drivers/gpu/drm/drm_fb_helper.c | 8 ++- > > > drivers/gpu/drm/drm_gem.c | 108 +-- > > > drivers/gpu/drm/drm_prime.c | 40 ++-- > > > include/drm/drm_gem.h | 138 > > > > > > 5 files changed, 272 insertions(+), 34 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > > > index 17d9a64e885e..eca7331762e4 100644 > > > --- a/drivers/gpu/drm/drm_client.c > > > +++ b/drivers/gpu/drm/drm_client.c > > > @@ -80,8 +80,7 @@ int drm_client_new(struct drm_device *dev, struct > > > drm_client_dev *client, > > > { > > > int ret; > > > - if (!drm_core_check_feature(dev, DRIVER_MODESET) || > > > - !dev->driver->dumb_create || !dev->driver->gem_prime_vmap) > > > + if (!drm_core_check_feature(dev, DRIVER_MODESET) || > > > !dev->driver->dumb_create) > > > return -EOPNOTSUPP; > > I guess the ->gem_prime_vmap check got lost, needs to be readded somewhere > > I think. > > It happens in drm_client_buffer_create() when drm_gem_vmap() is called. > It returns -EOPNOTSUPP if no callback is set. Ah, missed that. > > > > > if (funcs && !try_module_get(funcs->owner)) > > > @@ -212,8 +211,7 @@ static void drm_client_buffer_delete(struct > > > drm_client_buffer *buffer) > > > { > > > struct drm_device *dev = buffer->client->dev; > > > - if (buffer->vaddr && dev->driver->gem_prime_vunmap) > > > - dev->driver->gem_prime_vunmap(buffer->gem, buffer->vaddr); > > > + drm_gem_vunmap(buffer->gem, buffer->vaddr); > > > if (buffer->gem) > > > drm_gem_object_put_unlocked(buffer->gem); > > > @@ -266,9 +264,9 @@ drm_client_buffer_create(struct drm_client_dev > > > *client, u32 width, u32 height, u > > >* fd_install step out of the driver backend hooks, to make that > > >* final step optional for internal users. > > >*/ > > > - vaddr = dev->driver->gem_prime_vmap(obj); > > > - if (!vaddr) { > > > - ret = -ENOMEM; > > > + vaddr = drm_gem_vmap(obj); > > > + if (IS_ERR(vaddr)) { > > > + ret = PTR_ERR(vaddr); > > > goto err_delete; > > > } > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > > b/drivers/gpu/drm/drm_fb_helper.c > > > index 8e95d0f7c71d..66969f0facbb 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -38,6 +38,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -3010,9 +3011,12 @@ static void drm_fbdev_fb_destroy(struct fb_info > > > *info) > > > static int drm_fbdev_fb_mmap(struct fb_info *info, struct > > > vm_area_struct *vma) > > > { > > > struct drm_fb_helper *fb_helper = info->par; > > > + struct drm_gem_object *obj = fb_helper->buffer->gem; > > > - if (fb_helper->dev->driver->gem_prime_mmap) > > > - return > > > fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma); > > > + if (obj->dev->driver->gem_prime_mmap) > > > + return obj->dev->driver->gem_prime_mmap(obj, vma); > > > + else if (obj->funcs && obj->funcs->mmap) > > > + return drm_gem_mmap_obj(obj, obj->size, vma); > > > else > > > return -ENODEV; > > > } > > > diff --git a/drivers/gpu/drm/drm_gem.c b
[Bug 108014] AMD WX4150 - MST is entirely nonfunctional, spams dmesg with errors
https://bugs.freedesktop.org/show_bug.cgi?id=108014 --- Comment #1 from Jerry Zuo --- The MST is working in the first modeset, and afterwards it stops working. Does it mean that each mode change after first modeset doesn'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: RFE: DRM_IOCTL_MODE_EXPOSE_LEASE
On Sat, Sep 22, 2018 at 04:34:03PM +0300, Troll Berserker wrote: > On 22/09/18 11:45, Daniel Vetter wrote: > > Hi, > > > > On Sat, Sep 22, 2018 at 05:55:00AM +0300, Troll Berserker wrote: > > > Goal: simplify multiseat support. > > > > Sounds like a neat idea to use leases for this. > > > A new parameter was added to the Xorg server to make it use a passed > > > file descriptor instead of /dev/dri/card*. This enables one to start > > > the Xorg server with leased FD. > > > Although it is possible to organize multiseat using this approach, it > > > does not integrate well with existing seat infrastructure (udev, > > > logind). > > > > Why does it not integrate well with logind? > > Without a /sys/ subtree for lease you can't create a new seat with > loginctl attach seat-X /sys/devices/... > logind filters non-graphic devices. > > > > It would be great to have a way to expose a DRM Lessee as /dev/dri/card* > > > node and relevant /sys nodes etc., which would enable one to write an > > > udev rule to create a new seat. This new node should represent leased > > > resources and *should support > > > DRM_IOCTL_SET_MASTER/DRM_IOCTL_DROP_MASTER* as if it were a "real" > > > device. > > > > Note that leases do not make _any_ guarantees about resource usage > > isolation. If one lease lights a high res screen and uses too much scanout > > bandwidth, the other lease won't be able to use it's output. This isn't > > really fixiable without rewriting all the drivers. The way it's solved is > > that the lessor can take away the lease anytime to shut up mis-behaving > > clients. So if you want multi-seat, you probably need someone to properly > > manage these leases for this reason. Not just dumb dev nodes.> The other > > issue with leases is that they're tied to the current owner > > (drm_master) of the device. If the owner switches, all leases switch too > > (and can't access the device anymore), e.g. when doing a vt-switch. As > > mentioned above, logind already does this, and compositors take their drm > > FD from logind. For multi-seat you probably don't want a user-switch on > > one seat to switch all seats, so I think this needs to be put into logind. > > I've studied the topic a bit and understand that there should be a daemon > starting before display managers holding master privileges for some > /dev/dri/card* device and managing leases. > For *multi*seat, one will have to create at least two leases because original > device would be busy. Yup, you need this already. Why can't it also handle multi-seat? Yes, logind needs to be changed, but I think you need to do that anyway. Because leases do _not_ give you perfect isolation between leases, so some oversight is needed. > > > Interface: DRM_IOCTL_MODE_EXPOSE_LEASE ioctl request to DRM Lessor with > > > lease FD as the ioctl parameter. > > > > Final issue I'm seeing: Kernel internals will pose some interesting > > lifetime issues, stuff like what happens when the lease is gone. Can you > > still open the device in that case? > > My view is that this should behave as if someone unplugged a USB-VGA dock. > I.e. the device shall disappear. > > Just first reactions, I think you need to supply more details here on what > > exactly the precise problem is you're trying to solve. > > The precise problem — organize multiseat with one GPU with multiple outputs. > Using logind infrastructure. > > I didn't try, but I think currently I would have to create seats artificially, > (because AFAIK display managers spawn a login screen per seat, not per screen) > for example, attaching /sys/devices/.../graphics/fb0 to the seat-1. > Or patch the logind to make it not filter card0/card0-HDMI-A-1 and other > /sys records for connectors. > Then write a xorg config defining layouts for seats. > And use intel driver with the ZaphodHeads option to be able to start > multiple Xorg servers at one GPU. > > With exposed leases this would be much easier to solve the task. I agree that leases sounds like a good tool here. I'm not sure that the ioctl you propose is the right thing to mesh this into logind. Afaik logind already wrestles drm device nodes with special code, so adding more special code for multi-seat shouldn't be a big deal. And would allow you to solve some of the problems I mentioned in a much cleaner fashion. E.g. you could have multi-user _and_ multi-seat and it would work correctly, with per-seat user switching. You can't do that with a kernel-only solution. Cheers, Daniel -- 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
[Bug 107607] Problems with MST display (REG_WAIT takes a while or times out)
https://bugs.freedesktop.org/show_bug.cgi?id=107607 --- Comment #7 from Nicholas Kazlauskas --- Can you clarify the problem you're observing? Do all the displays go blank after switching inputs? -- 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 v2] drm/arm/malidp: Validate rotations for compressed/uncompressed framebuffers for each layer
On Mon, Sep 24, 2018 at 03:40:15PM +0100, Ayan Halder wrote: > Hi Liviu, Hi Ayan, > > On Fri, Sep 21, 2018 at 03:33:53PM +0100, Liviu Dudau wrote: > > From: Ayan Kumar Halder > > > > Add support for compressed framebuffers that are described using > > the framebuffer's modifier field. Mali DP uses the rotation memory for > > the decompressor of the format, so we need to check for space when > > the modifiers are present. > > > > Signed-off-by: Ayan Kumar Halder > > Reviewed-by: Brian Starkey > > [re-worded commit, rebased, cleaned up duplicated checks for > > RGB888 and BGR888 and removed additional parameter for > > rotmem_required function hook] > > Signed-off-by: Liviu Dudau > > --- > > drivers/gpu/drm/arm/malidp_crtc.c | 28 - > > drivers/gpu/drm/arm/malidp_hw.c | 38 - > > drivers/gpu/drm/arm/malidp_hw.h | 7 ++ > > drivers/gpu/drm/arm/malidp_planes.c | 19 +++ > > 4 files changed, 53 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/gpu/drm/arm/malidp_crtc.c > > b/drivers/gpu/drm/arm/malidp_crtc.c > > index ef44202fb43f8..e1b72782848c3 100644 > > --- a/drivers/gpu/drm/arm/malidp_crtc.c > > +++ b/drivers/gpu/drm/arm/malidp_crtc.c > > @@ -348,19 +348,20 @@ static int malidp_crtc_atomic_check(struct drm_crtc > > *crtc, > > > > /* > > * check if there is enough rotation memory available for planes > > -* that need 90?? and 270?? rotation. Each plane has set its required > > -* memory size in the ->plane_check() callback, here we only make > > -* sure that the sums are less that the total usable memory. > > +* that need 90?? and 270?? rotion or planes that are compressed. > > +* Each plane has set its required memory size in the ->plane_check() > > +* callback, here we only make sure that the sums are less that the > > +* total usable memory. > > * > > * The rotation memory allocation algorithm (for each plane): > > -* a. If no more rotated planes exist, all remaining rotate > > -* memory in the bank is available for use by the plane. > > -* b. If other rotated planes exist, and plane's layer ID is > > -* DE_VIDEO1, it can use all the memory from first bank if > > -* secondary rotation memory bank is available, otherwise it can > > +* a. If no more rotated or compressed planes exist, all remaining > > +* rotate memory in the bank is available for use by the plane. > > +* b. If other rotated or compressed planes exist, and plane's > > +* layer ID is DE_VIDEO1, it can use all the memory from first bank > > +* if secondary rotation memory bank is available, otherwise it can > > * use up to half the bank's memory. > > -* c. If other rotated planes exist, and plane's layer ID is not > > -* DE_VIDEO1, it can use half of the available memory > > +* c. If other rotated or compressed planes exist, and plane's layer ID > > +* is not DE_VIDEO1, it can use half of the available memory. > > * > > * Note: this algorithm assumes that the order in which the planes are > > * checked always has DE_VIDEO1 plane first in the list if it is > > @@ -372,7 +373,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc > > *crtc, > > > > /* first count the number of rotated planes */ > > drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) { > > - if (pstate->rotation & MALIDP_ROTATED_MASK) > > + struct drm_framebuffer *fb = pstate->fb; > > + > > + if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier) > > rotated_planes++; > > } > > > > @@ -388,8 +391,9 @@ static int malidp_crtc_atomic_check(struct drm_crtc > > *crtc, > > drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) { > > struct malidp_plane *mp = to_malidp_plane(plane); > > struct malidp_plane_state *ms = to_malidp_plane_state(pstate); > > + struct drm_framebuffer *fb = pstate->fb; > > > > - if (pstate->rotation & MALIDP_ROTATED_MASK) { > > + if ((pstate->rotation & MALIDP_ROTATED_MASK) || fb->modifier) { > > /* process current plane */ > > rotated_planes--; > > > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c > > b/drivers/gpu/drm/arm/malidp_hw.c > > index b33000634a4ee..5549092e6c36a 100644 > > --- a/drivers/gpu/drm/arm/malidp_hw.c > > +++ b/drivers/gpu/drm/arm/malidp_hw.c > > @@ -85,41 +85,43 @@ static const struct malidp_format_id > > malidp550_de_formats[] = { > > > > static const struct malidp_layer malidp500_layers[] = { > > /* id, base address, fb pointer address base, stride offset, > > - yuv2rgb matrix offset, mmu control register offset */ > > + yuv2rgb matrix offset, mmu control register offset, > > rotation_features */ > > { DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_D
Re: Improve reservation object shared slot function
On Mon, Sep 24, 2018 at 01:58:12PM +0200, Christian König wrote: > The reservation object shared slot function only allowed to reserve one slot > at a time. > > Improve that and allow to reserve multiple slots to support atomically > submission to multiple engines. I think you can do this already, just don't drop the ww_mutex lock. And I also think that invariant still holds, if you drop the ww_mutex lock your fence slot reservation evaporates. Your new code is just a bit more convenient. Could we check/enforce this somehow when WW_MUTEX debugging is enabled? E.g. store the ww_mutex ctx in the reservation (we can dig it out from under the lock), and then check that the lock holder/ctx hasn't changed when adding all the fences? I think with multiple fences getting added atomically some more debug checks here would be good. Another one: Do we want to insist that you either add all the fences, or none, to make this fully atomic? We could check this when unlocking the reservation. Kinda hard to guess what your exact use-case here is. All this would ofc be compiled out for !WW_MUTEX_DEBUG kernels. Cheers, Daniel -- 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
Re: Improve reservation object shared slot function
Am 24.09.2018 um 17:03 schrieb Daniel Vetter: On Mon, Sep 24, 2018 at 01:58:12PM +0200, Christian König wrote: The reservation object shared slot function only allowed to reserve one slot at a time. Improve that and allow to reserve multiple slots to support atomically submission to multiple engines. I think you can do this already, just don't drop the ww_mutex lock. And I also think that invariant still holds, if you drop the ww_mutex lock your fence slot reservation evaporates. Your new code is just a bit more convenient. The problem is that allocating a slot could in theory fail with -ENOMEM. And at the point we add the fence the hardware is already using the memory, so failure is not on option. Key feature is that we are getting submissions to multiple engines which either needs to be submitted together or not at all. Could we check/enforce this somehow when WW_MUTEX debugging is enabled? E.g. store the ww_mutex ctx in the reservation (we can dig it out from under the lock), and then check that the lock holder/ctx hasn't changed when adding all the fences? I think with multiple fences getting added atomically some more debug checks here would be good. Yeah, I was already thinking about something similar. We wouldn't need to remember the context or anything, but rather just set shared_max=shared_count in reservation_object_unlock(). Another one: Do we want to insist that you either add all the fences, or none, to make this fully atomic? We could check this when unlocking the reservation. Kinda hard to guess what your exact use-case here is. No, some fence (like VM updates) are perfectly optional. All this would ofc be compiled out for !WW_MUTEX_DEBUG kernels. Those extra checks sounds like a good idea to me, going to add that. Thanks, Christian. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] meson: honor -Detnaviv=auto
On Friday, 2018-09-21 17:30:42 +0200, Guido Günther wrote: > We support that value so it should work as expected. This does not make > 'auto' the default since the API is still marked as experimental. > --- > meson.build | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/meson.build b/meson.build > index 75c7bdff..8001a9cc 100644 > --- a/meson.build > +++ b/meson.build > @@ -142,11 +142,11 @@ endif > > with_etnaviv = false > _etnaviv = get_option('etnaviv') > -if _etnaviv == 'true' > +if _etnaviv != 'false' >if not with_atomics > error('libdrm_etnaviv requires atomics.') We shouldn't error out in the 'auto' case; please modify the `with_atomic` if above to add `_etnaviv == 'true' and ...`. With that, the patch is: Reviewed-by: Eric Engestrom >endif > - with_etnaviv = true > + with_etnaviv = _etnaviv == 'true' or ['arm', > 'aarch64'].contains(host_machine.cpu_family()) That said, I have no idea if enabling it by default on ARM is the right thing, so I'll let Lucas and/or Christian decide this :) (You should wait for their reply before sending your v2) > endif > > with_exynos = get_option('exynos') == 'true' > -- > 2.18.0 > ___ > 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: Improve reservation object shared slot function
On Mon, Sep 24, 2018 at 05:16:50PM +0200, Christian König wrote: > Am 24.09.2018 um 17:03 schrieb Daniel Vetter: > > On Mon, Sep 24, 2018 at 01:58:12PM +0200, Christian König wrote: > > > The reservation object shared slot function only allowed to reserve one > > > slot at a time. > > > > > > Improve that and allow to reserve multiple slots to support atomically > > > submission to multiple engines. > > I think you can do this already, just don't drop the ww_mutex lock. And I > > also think that invariant still holds, if you drop the ww_mutex lock your > > fence slot reservation evaporates. Your new code is just a bit more > > convenient. > > The problem is that allocating a slot could in theory fail with -ENOMEM. > > And at the point we add the fence the hardware is already using the memory, > so failure is not on option. > > Key feature is that we are getting submissions to multiple engines which > either needs to be submitted together or not at all. > > > Could we check/enforce this somehow when WW_MUTEX debugging is enabled? > > E.g. store the ww_mutex ctx in the reservation (we can dig it out from > > under the lock), and then check that the lock holder/ctx hasn't changed > > when adding all the fences? I think with multiple fences getting added > > atomically some more debug checks here would be good. > > Yeah, I was already thinking about something similar. > > We wouldn't need to remember the context or anything, but rather just set > shared_max=shared_count in reservation_object_unlock(). > > > Another one: Do we want to insist that you either add all the fences, or > > none, to make this fully atomic? We could check this when unlocking the > > reservation. Kinda hard to guess what your exact use-case here is. > > No, some fence (like VM updates) are perfectly optional. Ah right, vm moves might already be underway (even if you end up throwing all the actual CS stuff from userspace away), or might not happen. > > All this would ofc be compiled out for !WW_MUTEX_DEBUG kernels. > > Those extra checks sounds like a good idea to me, going to add that. Yeah that'd be great, just figured "this has lots of potential for nasty to debug bugs". The patches themselves look good to me, at least from skimming. -Daniel -- 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
[Bug 108036] VA-API implementation reports support for unsupported endpoints
https://bugs.freedesktop.org/show_bug.cgi?id=108036 --- Comment #2 from Kurt Kartaltepe --- The VA-API implementation correctly reports b-frames are unavailable and in this test they were not used. The implemention also provided a decodable stream on main profile. So if the the firmware issue you describe affects main profile streams than this system is unaffected. And this is a separate issue. -- 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: [Intel-gfx] [PATCH 11/18] drm/i915: Return the mask of enabled infoframes from ->inforame_enabled()
On Thu, Sep 20, 2018 at 09:51:38PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > We want to start tracking which infoframes are enabled, so let's replace > the boolean flag with a bitmask. > > We'll abstract the bitmask so that it's not platform dependent. That > will allow us to examine the bitmask later in platform independent code. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > drivers/gpu/drm/i915/intel_hdmi.c | 87 > --- > 3 files changed, 68 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 086e3f940586..098a0e4edf2a 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3390,7 +3390,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder, > pipe_config->has_hdmi_sink = true; > intel_dig_port = enc_to_dig_port(&encoder->base); > > - if (intel_dig_port->infoframe_enabled(encoder, pipe_config)) > + if (intel_hdmi_infoframes_enabled(encoder, pipe_config)) > pipe_config->has_infoframe = true; > > if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) == > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index e0f3a79fc75e..6815c69aac2f 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1181,7 +1181,7 @@ struct intel_digital_port { > bool enable, > const struct intel_crtc_state *crtc_state, > const struct drm_connector_state *conn_state); > - bool (*infoframe_enabled)(struct intel_encoder *encoder, > + u32 (*infoframes_enabled)(struct intel_encoder *encoder, > const struct intel_crtc_state *pipe_config); > }; > > @@ -1856,6 +1856,8 @@ bool intel_hdmi_handle_sink_scrambling(struct > intel_encoder *encoder, > bool scrambling); > void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool > enable); > void intel_infoframe_init(struct intel_digital_port *intel_dig_port); > +u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > + const struct intel_crtc_state *crtc_state); > > > /* intel_lvds.c */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index c3c2a638d062..a8fcddb199ae 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -100,10 +100,14 @@ static u32 g4x_infoframe_index(unsigned int type) > static u32 g4x_infoframe_enable(unsigned int type) > { > switch (type) { > + case HDMI_PACKET_TYPE_NULL: > + return VIDEO_DIP_ENABLE; /* slight lie */ Not exactly sure why we're tracking this one here, but not for hsw. Shouldn't we include that one if the DDI port is in hdmi mode? Would be more consistent I think. Aside from that lgtm, has my Reviewed-by: Daniel Vetter once we figured the TYPE_NULL thing out. -Daniel > case HDMI_PACKET_TYPE_GENERAL_CONTROL: > return VIDEO_DIP_ENABLE_GCP; > case HDMI_PACKET_TYPE_GAMUT_METADATA: > return VIDEO_DIP_ENABLE_GAMUT; > + case DP_SDP_VSC: > + return 0; > case HDMI_INFOFRAME_TYPE_AVI: > return VIDEO_DIP_ENABLE_AVI; > case HDMI_INFOFRAME_TYPE_SPD: > @@ -119,6 +123,8 @@ static u32 g4x_infoframe_enable(unsigned int type) > static u32 hsw_infoframe_enable(unsigned int type) > { > switch (type) { > + case HDMI_PACKET_TYPE_NULL: > + return 0; > case HDMI_PACKET_TYPE_GENERAL_CONTROL: > return VIDEO_DIP_ENABLE_GCP_HSW; > case HDMI_PACKET_TYPE_GAMUT_METADATA: > @@ -197,19 +203,19 @@ static void g4x_write_infoframe(struct intel_encoder > *encoder, > POSTING_READ(VIDEO_DIP_CTL); > } > > -static bool g4x_infoframe_enabled(struct intel_encoder *encoder, > +static u32 g4x_infoframes_enabled(struct intel_encoder *encoder, > const struct intel_crtc_state *pipe_config) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > u32 val = I915_READ(VIDEO_DIP_CTL); > > if ((val & VIDEO_DIP_ENABLE) == 0) > - return false; > + return 0; > > if ((val & VIDEO_DIP_PORT_MASK) != VIDEO_DIP_PORT(encoder->port)) > - return false; > + return 0; > > - return val & (VIDEO_DIP_ENABLE_AVI | > + return val & (VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI | > VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_SPD); > } > > @@ -252,7 +258,7 @@ static void ibx_write_infoframe(struct intel_encoder > *encoder, > POSTING_READ(reg); > } > > -static bool ibx_infoframe_enabled(struc
Re: [Intel-gfx] [PATCH 12/18] drm/i915: Store mask of enabled infoframes in the crtc state
On Thu, Sep 20, 2018 at 09:51:39PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Store the mask of enabled infoframes in the crtc state. We'll start > with just the readout for HDMI encoder, and we'll expand this > to compute the bitmask in .compute_config() later. SDVO will also > follow later. > > Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_ddi.c | 5 - > drivers/gpu/drm/i915/intel_drv.h | 4 > drivers/gpu/drm/i915/intel_hdmi.c | 5 - > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 098a0e4edf2a..19fef88e680e 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3390,7 +3390,10 @@ void intel_ddi_get_config(struct intel_encoder > *encoder, > pipe_config->has_hdmi_sink = true; > intel_dig_port = enc_to_dig_port(&encoder->base); > > - if (intel_hdmi_infoframes_enabled(encoder, pipe_config)) > + pipe_config->infoframes.enable |= > + intel_hdmi_infoframes_enabled(encoder, pipe_config); > + > + if (pipe_config->infoframes.enable) > pipe_config->has_infoframe = true; > > if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) == > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 6815c69aac2f..50c0c049ee15 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -893,6 +893,10 @@ struct intel_crtc_state { > u8 active_planes; > u8 nv12_planes; > > + struct { > + u32 enable; > + } infoframes; > + > /* HDMI scrambling status */ > bool hdmi_scrambling; > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index a8fcddb199ae..98a44084324c 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1264,7 +1264,10 @@ static void intel_hdmi_get_config(struct intel_encoder > *encoder, > if (tmp & HDMI_MODE_SELECT_HDMI) > pipe_config->has_hdmi_sink = true; > > - if (intel_hdmi_infoframes_enabled(encoder, pipe_config)) > + pipe_config->infoframes.enable |= > + intel_hdmi_infoframes_enabled(encoder, pipe_config); > + > + if (pipe_config->infoframes.enable) > pipe_config->has_infoframe = true; > > if (tmp & SDVO_AUDIO_ENABLE) > -- > 2.16.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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
Re: [Intel-gfx] [PATCH 13/18] drm/i915: Precompute HDMI infoframes
On Thu, Sep 20, 2018 at 09:51:40PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Store the infoframes in the crtc state and precompute them in > .compute_config(). While precomputing we'll also fill out the > inforames.enable bitmask appropriately. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_ddi.c | 1 + > drivers/gpu/drm/i915/intel_drv.h | 5 + > drivers/gpu/drm/i915/intel_hdmi.c | 249 > +++--- > 3 files changed, 187 insertions(+), 68 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 19fef88e680e..5f3bd536d261 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3391,6 +3391,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder, > intel_dig_port = enc_to_dig_port(&encoder->base); > > pipe_config->infoframes.enable |= > + intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_NULL) | Misplaced hunk? Assuming I'm reading this correctly, this will give you a 0. Not exactly sure what's going on here ... I guess I'm not following why we care about TYPE_NULL, and why we have to reconstruct it here? I thought TYPE_NULL is equivalent to state->has_hdmi_sink? Comment (unfortunately not yet kerneldoc) even explains that ... Maybe just nuke all the TYPE_NULL tracking here, perhaps with the g4x (except for g4x itself, because it's shared there) decoder to also take DIP_ENABLE into account for has_hdmi_sink. Or update the comment in intel_crtc_state. Otherwise lgtm, thought admittedly I did clean over the details a bit, trusting CI and gcc to catch the small stuff :-) Reviewed-by: Daniel Vetter with the TYPE_NULL story somehow figured out. -Daniel > intel_hdmi_infoframes_enabled(encoder, pipe_config); > > if (pipe_config->infoframes.enable) > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 50c0c049ee15..357624a6bfe2 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -895,6 +895,10 @@ struct intel_crtc_state { > > struct { > u32 enable; > + u32 gcp; > + union hdmi_infoframe avi; > + union hdmi_infoframe spd; > + union hdmi_infoframe hdmi; > } infoframes; > > /* HDMI scrambling status */ > @@ -1862,6 +1866,7 @@ void intel_dp_dual_mode_set_tmds_output(struct > intel_hdmi *hdmi, bool enable); > void intel_infoframe_init(struct intel_digital_port *intel_dig_port); > u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state); > +u32 intel_hdmi_infoframe_enable(unsigned int type); > > > /* intel_lvds.c */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 98a44084324c..491001fc0fad 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -446,6 +446,18 @@ static const u8 infoframe_type_to_idx[] = { > HDMI_INFOFRAME_TYPE_VENDOR, > }; > > +u32 intel_hdmi_infoframe_enable(unsigned int type) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(infoframe_type_to_idx); i++) { > + if (infoframe_type_to_idx[i] == type) > + return BIT(i); > + } > + > + return 0; > +} > + > u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state) > { > @@ -491,15 +503,23 @@ u32 intel_hdmi_infoframes_enabled(struct intel_encoder > *encoder, > */ > static void intel_write_infoframe(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state, > - union hdmi_infoframe *frame) > + enum hdmi_infoframe_type type, > + const union hdmi_infoframe *frame) > { > struct intel_digital_port *intel_dig_port = > enc_to_dig_port(&encoder->base); > u8 buffer[VIDEO_DIP_DATA_SIZE]; > ssize_t len; > > + if ((crtc_state->infoframes.enable & > + intel_hdmi_infoframe_enable(type)) == 0) > + return; > + > + if (WARN_ON(frame->any.type != type)) > + return; > + > /* see comment above for the reason for this offset */ > - len = hdmi_infoframe_pack(frame, buffer + 1, sizeof(buffer) - 1); > - if (len < 0) > + len = hdmi_infoframe_pack_only(frame, buffer + 1, sizeof(buffer) - 1); > + if (WARN_ON(len < 0)) > return; > > /* Insert the 'hole' (see big comment above) at position 3 */ > @@ -507,85 +527,111 @@ static void intel_write_infoframe(struct intel_encoder > *encoder, > buffer[3] = 0; > len++; > > - intel_dig_port->write_infoframe(encoder, > -
Re: [Intel-gfx] [PATCH 14/18] drm/i915: Read out HDMI infoframes
On Thu, Sep 20, 2018 at 09:51:41PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Add code to read the infoframes from the video DIP and unpack them into > the crtc state. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_ddi.c | 17 > drivers/gpu/drm/i915/intel_drv.h | 10 ++ > drivers/gpu/drm/i915/intel_hdmi.c | 203 > ++ > 3 files changed, 230 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 5f3bd536d261..a56289f78326 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3459,6 +3459,23 @@ void intel_ddi_get_config(struct intel_encoder > *encoder, > bxt_ddi_phy_get_lane_lat_optim_mask(encoder); > > intel_ddi_compute_min_voltage_level(dev_priv, pipe_config); > + > + intel_hdmi_read_gcp_infoframe(encoder, pipe_config); > + > + if (!intel_read_infoframe(encoder, pipe_config, > + HDMI_INFOFRAME_TYPE_AVI, > + &pipe_config->infoframes.avi)) > + DRM_ERROR("failed to read AVI infoframe\n"); > + > + if (!intel_read_infoframe(encoder, pipe_config, > + HDMI_INFOFRAME_TYPE_SPD, > + &pipe_config->infoframes.spd)) > + DRM_ERROR("failed to read SPD infoframe:\n"); > + > + if (!intel_read_infoframe(encoder, pipe_config, > + HDMI_INFOFRAME_TYPE_VENDOR, > + &pipe_config->infoframes.hdmi)) > + DRM_ERROR("failed to read HDMI infoframe\n"); > } > > static enum intel_output_type > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 357624a6bfe2..75ec99b85232 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1185,6 +1185,10 @@ struct intel_digital_port { > const struct intel_crtc_state *crtc_state, > unsigned int type, > const void *frame, ssize_t len); > + ssize_t (*read_infoframe)(struct intel_encoder *encoder, > + const struct intel_crtc_state *crtc_state, > + unsigned int type, > + void *frame, ssize_t len); > void (*set_infoframes)(struct intel_encoder *encoder, > bool enable, > const struct intel_crtc_state *crtc_state, > @@ -1867,6 +1871,12 @@ void intel_infoframe_init(struct intel_digital_port > *intel_dig_port); > u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state); > u32 intel_hdmi_infoframe_enable(unsigned int type); > +void intel_hdmi_read_gcp_infoframe(struct intel_encoder *encoder, > +struct intel_crtc_state *crtc_state); > +bool intel_read_infoframe(struct intel_encoder *encoder, > + const struct intel_crtc_state *crtc_state, > + enum hdmi_infoframe_type type, > + union hdmi_infoframe *frame); > > > /* intel_lvds.c */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 491001fc0fad..27cb6ec32e94 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -203,6 +203,31 @@ static void g4x_write_infoframe(struct intel_encoder > *encoder, > POSTING_READ(VIDEO_DIP_CTL); > } > > +static ssize_t g4x_read_infoframe(struct intel_encoder *encoder, > + const struct intel_crtc_state *crtc_state, > + unsigned int type, > + void *frame, ssize_t len) > +{ > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + u32 val, *data = frame; > + int i; > + > + val = I915_READ(VIDEO_DIP_CTL); > + > + if ((val & g4x_infoframe_enable(type)) == 0) > + return 0; > + > + val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ > + val |= g4x_infoframe_index(type); > + > + I915_WRITE(VIDEO_DIP_CTL, val); > + > + for (i = 0; i < len; i += 4) > + *data++ = I915_READ(VIDEO_DIP_DATA); > + > + return len; > +} > + > static u32 g4x_infoframes_enabled(struct intel_encoder *encoder, > const struct intel_crtc_state *pipe_config) > { > @@ -258,6 +283,32 @@ static void ibx_write_infoframe(struct intel_encoder > *encoder, > POSTING_READ(reg); > } > > +static ssize_t ibx_read_infoframe(struct intel_encoder *encoder, > + const struct intel_crtc_state *crtc_state, > + unsigned int type, > + void *frame
[Bug 108036] VA-API implementation reports support for unsupported endpoints
https://bugs.freedesktop.org/show_bug.cgi?id=108036 --- Comment #3 from Kurt Kartaltepe --- > The hardware supports some high profile features (like CABAC), but > unfortunately not all of them (like B-frames or MBAFF). > > Now the profile selects what the decoder needs to be able to do to handle a > certain video, but doesn't tells you anything about the encoder except for > selecting the encoding of the headers. > > We should support the encoding of the headers, so if an application selects > high profile it actually gets better compression because of CABAC support. > > But if the application also tries to use B-frames it will get an invalid > stream. Yes, the issue is that valid settings produce a stream that is invalid. Personally I would be more than ok if the resultant stream on high was within constrained baseline specs as long as it returned a valid stream. Because hardware encoders are such black boxes I care very little for the settings i cannot control (such as exact compression techniques) and expect the ones i can to 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: [Intel-gfx] [PATCH 16/18] drm/i915/sdvo: Read out HDMI infoframes
On Thu, Sep 20, 2018 at 09:51:43PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Read the HDMI infoframes from the hbuf and unpack them into > the crtc state. > > Well, actually just AVI infoframe for now but let's write the > infoframe readout code in a more generic fashion in case we > expand this later. > > Signed-off-by: Ville Syrjälä Hm, caring about sdvo seems a bit overkill. And afaik we don't have any sdvo (much less hdmi) in CI. I'm leaning towards just adding a PIPE_CONFIG_QUIRK_INFOFRAMES for sdvo, and short-circuiting the checks if that's set. Except if you can somehow convince CI folks to add an sdvo hdmi card to CI :-) > --- > drivers/gpu/drm/i915/intel_sdvo.c | 92 > +-- > 1 file changed, 89 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > b/drivers/gpu/drm/i915/intel_sdvo.c > index d8c78aebaf01..4d787c86df6d 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -981,6 +981,58 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo > *intel_sdvo, > &tx_rate, 1); > } > > +static ssize_t intel_sdvo_read_infoframe(struct intel_sdvo *intel_sdvo, > + unsigned int if_index, > + u8 *data, unsigned int length) > +{ > + u8 set_buf_index[2] = { if_index, 0 }; > + u8 hbuf_size, tx_rate, av_split; > + int i; > + > + if (!intel_sdvo_get_value(intel_sdvo, > + SDVO_CMD_GET_HBUF_AV_SPLIT, > + &av_split, 1)) > + return -ENXIO; > + > + if (av_split < if_index) > + return 0; > + > + if (!intel_sdvo_get_value(intel_sdvo, > + SDVO_CMD_GET_HBUF_TXRATE, > + &tx_rate, 1)) > + return -ENXIO; > + > + if (tx_rate == SDVO_HBUF_TX_DISABLED) > + return 0; > + > + if (!intel_sdvo_set_value(intel_sdvo, > + SDVO_CMD_SET_HBUF_INDEX, > + set_buf_index, 2)) > + return -ENXIO; > + > + if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HBUF_INFO, > + &hbuf_size, 1)) > + return -ENXIO; > + > + /* Buffer size is 0 based, hooray! */ > + hbuf_size++; > + > + DRM_DEBUG_KMS("reading sdvo hbuf: %i, hbuf_size %i, hbuf_size: %i\n", > + if_index, length, hbuf_size); > + > + hbuf_size = min_t(unsigned int, length, hbuf_size); > + > + for (i = 0; i < hbuf_size; i += 8) { > + if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_HBUF_DATA, > NULL, 0)) > + return -ENXIO; > + if (!intel_sdvo_read_response(intel_sdvo, &data[i], > + min_t(unsigned int, 8, hbuf_size > - i))) > + return -ENXIO; > + } > + > + return hbuf_size; > +} > + > static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo, >struct intel_crtc_state > *crtc_state, >struct drm_connector_state > *conn_state) > @@ -1039,6 +1091,37 @@ static bool intel_sdvo_set_avi_infoframe(struct > intel_sdvo *intel_sdvo, > sdvo_data, sizeof(sdvo_data)); > } > > +static bool intel_sdvo_get_avi_infoframe(struct intel_sdvo *intel_sdvo, > + struct intel_crtc_state *crtc_state) > +{ > + u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)]; > + union hdmi_infoframe *frame = &crtc_state->infoframes.avi; > + ssize_t len; > + int ret; > + > + if (!crtc_state->has_hdmi_sink) > + return true; > + > + len = intel_sdvo_read_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF, > + sdvo_data, sizeof(sdvo_data)); > + if (len < 0) > + return false; > + else if (len == 0) > + return true; > + > + crtc_state->infoframes.enable |= > + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI); > + > + ret = hdmi_infoframe_unpack(frame, sdvo_data, sizeof(sdvo_data)); > + if (ret) > + return false; > + > + if (frame->any.type != HDMI_INFOFRAME_TYPE_AVI) > + return false; > + > + return true; > +} > + > static bool intel_sdvo_set_tv_format(struct intel_sdvo *intel_sdvo, >const struct drm_connector_state > *conn_state) > { > @@ -1535,6 +1618,10 @@ static void intel_sdvo_get_config(struct intel_encoder > *encoder, > } > } > > + WARN(encoder_pixel_multiplier != pipe_config->pixel_multiplier, > + "SDVO pixel multiplier mismatch, port: %i, encoder: %i\n", > + pipe_config->pixel_multiplier, encoder_pixel_multiplier); > + >
Re: [Intel-gfx] [PATCH 17/18] drm/i915: Check infoframe state in intel_pipe_config_compare()
On Thu, Sep 20, 2018 at 09:51:44PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Check the infoframes and infoframe enable state when comparing two > crtc states. > > We'll use the infoframe logging functions from video/hdmi.c to > show the infoframes as part of the state dump. > > TODO: Try to better integrate the infoframe dumps with > drm state dumps > > v2: drm_printk() is no more > > Signed-off-by: Ville Syrjälä > --- Might need adapting to PIPE_CONFIG_QUIRK_INFOFRAME, but aside from that Reviewed-by: Daniel Vetter > drivers/gpu/drm/i915/intel_display.c | 49 > +++- > 1 file changed, 48 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index fbcc56caffb6..3dce49e36a05 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11380,6 +11380,37 @@ intel_compare_link_m_n(const struct intel_link_m_n > *m_n, > return false; > } > > +static bool > +intel_compare_infoframe(const union hdmi_infoframe *a, > + const union hdmi_infoframe *b) > +{ > + return memcmp(a, b, sizeof(*a)) == 0; > +} > + > +static void > +pipe_config_infoframe_err(struct drm_i915_private *dev_priv, > + bool adjust, const char *name, > + const union hdmi_infoframe *a, > + const union hdmi_infoframe *b) > +{ > + if (adjust) { > + if ((drm_debug & DRM_UT_KMS) == 0) > + return; > + > + drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name); > + drm_dbg(DRM_UT_KMS, "expected:"); > + hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a); > + drm_dbg(DRM_UT_KMS, "found"); > + hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, b); > + } else { > + drm_err("mismatch in %s infoframe", name); > + drm_err("expected:"); > + hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, a); > + drm_err("found"); > + hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, b); > + } Mildly concerned about padding fields (since these are the not-compatified structs). Maybe dump the mismatching byte too, plus byte offset? Or maybe I'm just too paranoid. > +} > + > static void __printf(3, 4) > pipe_config_err(bool adjust, const char *name, const char *format, ...) > { > @@ -11541,7 +11572,17 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > } \ > } while (0) > > -#define PIPE_CONF_QUIRK(quirk) \ > +#define PIPE_CONF_CHECK_INFOFRAME(name) do { \ > + if (!intel_compare_infoframe(¤t_config->infoframes.name, \ > + &pipe_config->infoframes.name)) { \ > + pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \ > + ¤t_config->infoframes.name, \ > + &pipe_config->infoframes.name); \ > + ret = false; \ > + } \ > +} while (0) > + > +#define PIPE_CONF_QUIRK(quirk) \ > ((current_config->quirks | pipe_config->quirks) & (quirk)) > > PIPE_CONF_CHECK_I(cpu_transcoder); > @@ -11670,6 +11711,12 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > > PIPE_CONF_CHECK_I(min_voltage_level); > > + PIPE_CONF_CHECK_X(infoframes.enable); > + PIPE_CONF_CHECK_X(infoframes.gcp); > + PIPE_CONF_CHECK_INFOFRAME(avi); > + PIPE_CONF_CHECK_INFOFRAME(spd); > + PIPE_CONF_CHECK_INFOFRAME(hdmi); > + > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I > #undef PIPE_CONF_CHECK_BOOL > -- > 2.16.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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
Re: [Intel-gfx] [PATCH 18/18] drm/i915: Include infoframes in the crtc state dump
On Thu, Sep 20, 2018 at 09:51:45PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > Dump out the infoframes in the normal crtc state dump. > > TODO: Try to better integrate the infoframe dumps with > drm state dumps > > Signed-off-by: Ville Syrjälä Going to make dmesg with state debugging enabled even more noisier, but hey, whatever gives us more data to drown in :-) More seriously, maybe eventually someone ports drm debug over to one of the more scalable logging thingies. Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 26 ++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 3dce49e36a05..27ac33a2a4d3 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10924,6 +10924,16 @@ intel_dump_m_n_config(struct intel_crtc_state > *pipe_config, char *id, > m_n->link_m, m_n->link_n, m_n->tu); > } > > +static void > +intel_dump_infoframe(struct drm_i915_private *dev_priv, > + const union hdmi_infoframe *frame) > +{ > + if ((drm_debug & DRM_UT_KMS) == 0) > + return; > + > + hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, frame); > +} > + > #define OUTPUT_TYPE(x) [INTEL_OUTPUT_ ## x] = #x > > static const char * const output_type_str[] = { > @@ -11013,6 +11023,22 @@ static void intel_dump_pipe_config(struct intel_crtc > *crtc, > DRM_DEBUG_KMS("audio: %i, infoframes: %i\n", > pipe_config->has_audio, pipe_config->has_infoframe); > > + DRM_DEBUG_KMS("infoframes enabled: 0x%x\n", > + pipe_config->infoframes.enable); > + > + if (pipe_config->infoframes.enable & > + intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL)) > + DRM_DEBUG_KMS("GCP: 0x%x\n", pipe_config->infoframes.gcp); > + if (pipe_config->infoframes.enable & > + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) > + intel_dump_infoframe(dev_priv, &pipe_config->infoframes.avi); > + if (pipe_config->infoframes.enable & > + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_SPD)) > + intel_dump_infoframe(dev_priv, &pipe_config->infoframes.spd); > + if (pipe_config->infoframes.enable & > + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_VENDOR)) > + intel_dump_infoframe(dev_priv, &pipe_config->infoframes.hdmi); > + > DRM_DEBUG_KMS("requested mode:\n"); > drm_mode_debug_printmodeline(&pipe_config->base.mode); > DRM_DEBUG_KMS("adjusted mode:\n"); > -- > 2.16.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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
Re: [Intel-gfx] [PATCH 11/18] drm/i915: Return the mask of enabled infoframes from ->inforame_enabled()
On Mon, Sep 24, 2018 at 05:51:16PM +0200, Daniel Vetter wrote: > On Thu, Sep 20, 2018 at 09:51:38PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > We want to start tracking which infoframes are enabled, so let's replace > > the boolean flag with a bitmask. > > > > We'll abstract the bitmask so that it's not platform dependent. That > > will allow us to examine the bitmask later in platform independent code. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > > drivers/gpu/drm/i915/intel_drv.h | 4 +- > > drivers/gpu/drm/i915/intel_hdmi.c | 87 > > --- > > 3 files changed, 68 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 086e3f940586..098a0e4edf2a 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -3390,7 +3390,7 @@ void intel_ddi_get_config(struct intel_encoder > > *encoder, > > pipe_config->has_hdmi_sink = true; > > intel_dig_port = enc_to_dig_port(&encoder->base); > > > > - if (intel_dig_port->infoframe_enabled(encoder, pipe_config)) > > + if (intel_hdmi_infoframes_enabled(encoder, pipe_config)) > > pipe_config->has_infoframe = true; > > > > if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) == > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index e0f3a79fc75e..6815c69aac2f 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1181,7 +1181,7 @@ struct intel_digital_port { > >bool enable, > >const struct intel_crtc_state *crtc_state, > >const struct drm_connector_state *conn_state); > > - bool (*infoframe_enabled)(struct intel_encoder *encoder, > > + u32 (*infoframes_enabled)(struct intel_encoder *encoder, > > const struct intel_crtc_state *pipe_config); > > }; > > > > @@ -1856,6 +1856,8 @@ bool intel_hdmi_handle_sink_scrambling(struct > > intel_encoder *encoder, > >bool scrambling); > > void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool > > enable); > > void intel_infoframe_init(struct intel_digital_port *intel_dig_port); > > +u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > > + const struct intel_crtc_state *crtc_state); > > > > > > /* intel_lvds.c */ > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > > b/drivers/gpu/drm/i915/intel_hdmi.c > > index c3c2a638d062..a8fcddb199ae 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -100,10 +100,14 @@ static u32 g4x_infoframe_index(unsigned int type) > > static u32 g4x_infoframe_enable(unsigned int type) > > { > > switch (type) { > > + case HDMI_PACKET_TYPE_NULL: > > + return VIDEO_DIP_ENABLE; /* slight lie */ > > Not exactly sure why we're tracking this one here, but not for hsw. HSW+ doesn't have a DIP enable bit like this. It only has the bits to enable specific infoframes. > Shouldn't we include that one if the DDI port is in hdmi mode? Would be > more consistent I think. Yes that would seem like the more correct thing. I think the reason I did this here was so that I could map the DIP_ENABLE bit to something unique. Would allow us to differentiate between the "DIP enabled with no infoframes enabled" vs. "DIP enabled with some infoframes enabled" cases. But seeing as we always enable some infoframes I guess this doesn't really provide us with anything particularly useful. That said, I'm actually not sure whether the hw will send the null packets if we don't enable the DIP. Would require a HDMI analyzer to confirm. Hmm. Actually gen4 bspec tells me: "If DIP is enabled but DIP types are all disabled, no DIP is sent. However, a single Null DIP will be sent at the same point in the stream that DIP packets would have been sent. This is done to keep the port in HDMI mode, otherwise it would revert to DVI mode. The "Null packets enabled during vsync" mode (bit #9 of port control register) overrides this behavior." So I guess mapping the null packet to the DIP enable bit is more or less correct. Although the spec doesn't quite say whether the null packet is also sent when some DIP types are also enabled, or if it is only send when no DIP types are enabled. So to match the hw I guess the readout should really do something like: if (hdmi & HDMI_MODE || dip_ctl & DIP_ENABLE) infoframes |= TYPE_NULL; but that would again mean that we can't tell the two cases apart. > > Aside from that lgtm, has my > > Reviewed-by: Daniel Vetter > > once we figured the TYPE_NULL thing out. > -Daniel > > > case HDMI_PACKET_TYPE_GENERAL_CONTROL: > >
Re: [Intel-gfx] [PATCH 13/18] drm/i915: Precompute HDMI infoframes
On Mon, Sep 24, 2018 at 05:58:25PM +0200, Daniel Vetter wrote: > On Thu, Sep 20, 2018 at 09:51:40PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Store the infoframes in the crtc state and precompute them in > > .compute_config(). While precomputing we'll also fill out the > > inforames.enable bitmask appropriately. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 1 + > > drivers/gpu/drm/i915/intel_drv.h | 5 + > > drivers/gpu/drm/i915/intel_hdmi.c | 249 > > +++--- > > 3 files changed, 187 insertions(+), 68 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 19fef88e680e..5f3bd536d261 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -3391,6 +3391,7 @@ void intel_ddi_get_config(struct intel_encoder > > *encoder, > > intel_dig_port = enc_to_dig_port(&encoder->base); > > > > pipe_config->infoframes.enable |= > > + intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_NULL) | > > Misplaced hunk? Assuming I'm reading this correctly, this will give you a > 0. Not exactly sure what's going on here ... I guess I'm not following why > we care about TYPE_NULL, and why we have to reconstruct it here? I rather wanted the infoframes bitmask to be truthful about which packets we're sending out. But not quite sure why I included it in this particular patch. > > I thought TYPE_NULL is equivalent to state->has_hdmi_sink? Comment > (unfortunately not yet kerneldoc) even explains that ... Yeah it's the same thing (apart from the g4x DIP enable thing). Maybe I should remove has_hdmi_sink entirely and just rely on the null packet bit instead... > > Maybe just nuke all the TYPE_NULL tracking here, perhaps with the g4x > (except for g4x itself, because it's shared there) decoder to also take > DIP_ENABLE into account for has_hdmi_sink. That would the other option I suppose. Though I might like the idea of dropping the bool for the bitmask a bit more perhaps. > > Or update the comment in intel_crtc_state. > > Otherwise lgtm, thought admittedly I did clean over the details a bit, > trusting CI and gcc to catch the small stuff :-) > > Reviewed-by: Daniel Vetter with the TYPE_NULL > story somehow figured out. > -Daniel > > > > intel_hdmi_infoframes_enabled(encoder, pipe_config); > > > > if (pipe_config->infoframes.enable) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 50c0c049ee15..357624a6bfe2 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -895,6 +895,10 @@ struct intel_crtc_state { > > > > struct { > > u32 enable; > > + u32 gcp; > > + union hdmi_infoframe avi; > > + union hdmi_infoframe spd; > > + union hdmi_infoframe hdmi; > > } infoframes; > > > > /* HDMI scrambling status */ > > @@ -1862,6 +1866,7 @@ void intel_dp_dual_mode_set_tmds_output(struct > > intel_hdmi *hdmi, bool enable); > > void intel_infoframe_init(struct intel_digital_port *intel_dig_port); > > u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > > const struct intel_crtc_state *crtc_state); > > +u32 intel_hdmi_infoframe_enable(unsigned int type); > > > > > > /* intel_lvds.c */ > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > > b/drivers/gpu/drm/i915/intel_hdmi.c > > index 98a44084324c..491001fc0fad 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -446,6 +446,18 @@ static const u8 infoframe_type_to_idx[] = { > > HDMI_INFOFRAME_TYPE_VENDOR, > > }; > > > > +u32 intel_hdmi_infoframe_enable(unsigned int type) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(infoframe_type_to_idx); i++) { > > + if (infoframe_type_to_idx[i] == type) > > + return BIT(i); > > + } > > + > > + return 0; > > +} > > + > > u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > > const struct intel_crtc_state *crtc_state) > > { > > @@ -491,15 +503,23 @@ u32 intel_hdmi_infoframes_enabled(struct > > intel_encoder *encoder, > > */ > > static void intel_write_infoframe(struct intel_encoder *encoder, > > const struct intel_crtc_state *crtc_state, > > - union hdmi_infoframe *frame) > > + enum hdmi_infoframe_type type, > > + const union hdmi_infoframe *frame) > > { > > struct intel_digital_port *intel_dig_port = > > enc_to_dig_port(&encoder->base); > > u8 buffer[VIDEO_DIP_DATA_SIZE]; > > ssize_t len; > > > > + if ((crtc_state->infoframes.enable & > > +intel_hdmi_infoframe_enable(type)) == 0) > >
[Bug 108037] Turning monitors off and on again makes the kernel panic and system freeze
https://bugs.freedesktop.org/show_bug.cgi?id=108037 --- Comment #3 from Öyvind Saether --- (In reply to Michel Dänzer from comment #1) > Can you bisect? > P.S. FYI, this is what's called an oops, not a panic. oops, then. yes I now know how to bisect. I can. (In reply to Alex Deucher from comment #2) > When you say turn them off/on, do you mean via software (e.g., dpms) or via > the switch on the monitor? I used the actual power button on the monitors that have them and the stupid joystick thing on the third monitor (works like a button) when this first happens but I just found that it doesn't matter, triggered it on 4.18.9 with dpms too. On 4.18.9: > set xscreensaver to immediately turn monitors off with dpms > lock screen > wait for monitors go into suspend > press button on keyboard > monitors don't come back on, system freeze will attach same-problem-4.18.9.txt with that one. 4.18.5 is fine. I notice there was a lot of amdgpu changes between 4.18.7 and 4.18.8 in the log. I have used 4.19 git kernels for some time and this didn't happen to me until 4.19rc4 but that could simply be because I didn't turn off the monitors(???). bisecting will take 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 108037] Turning monitors off and on again makes the kernel panic and system freeze
https://bugs.freedesktop.org/show_bug.cgi?id=108037 --- Comment #4 from Öyvind Saether --- Created attachment 141716 --> https://bugs.freedesktop.org/attachment.cgi?id=141716&action=edit Found that 4.18.9 has the same problem. -- 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: [Intel-gfx] [PATCH 14/18] drm/i915: Read out HDMI infoframes
On Mon, Sep 24, 2018 at 06:08:09PM +0200, Daniel Vetter wrote: > On Thu, Sep 20, 2018 at 09:51:41PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Add code to read the infoframes from the video DIP and unpack them into > > the crtc state. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 17 > > drivers/gpu/drm/i915/intel_drv.h | 10 ++ > > drivers/gpu/drm/i915/intel_hdmi.c | 203 > > ++ > > 3 files changed, 230 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 5f3bd536d261..a56289f78326 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -3459,6 +3459,23 @@ void intel_ddi_get_config(struct intel_encoder > > *encoder, > > bxt_ddi_phy_get_lane_lat_optim_mask(encoder); > > > > intel_ddi_compute_min_voltage_level(dev_priv, pipe_config); > > + > > + intel_hdmi_read_gcp_infoframe(encoder, pipe_config); > > + > > + if (!intel_read_infoframe(encoder, pipe_config, > > + HDMI_INFOFRAME_TYPE_AVI, > > + &pipe_config->infoframes.avi)) > > + DRM_ERROR("failed to read AVI infoframe\n"); > > + > > + if (!intel_read_infoframe(encoder, pipe_config, > > + HDMI_INFOFRAME_TYPE_SPD, > > + &pipe_config->infoframes.spd)) > > + DRM_ERROR("failed to read SPD infoframe:\n"); > > + > > + if (!intel_read_infoframe(encoder, pipe_config, > > + HDMI_INFOFRAME_TYPE_VENDOR, > > + &pipe_config->infoframes.hdmi)) > > + DRM_ERROR("failed to read HDMI infoframe\n"); > > } > > > > static enum intel_output_type > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 357624a6bfe2..75ec99b85232 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1185,6 +1185,10 @@ struct intel_digital_port { > > const struct intel_crtc_state *crtc_state, > > unsigned int type, > > const void *frame, ssize_t len); > > + ssize_t (*read_infoframe)(struct intel_encoder *encoder, > > + const struct intel_crtc_state *crtc_state, > > + unsigned int type, > > + void *frame, ssize_t len); > > void (*set_infoframes)(struct intel_encoder *encoder, > >bool enable, > >const struct intel_crtc_state *crtc_state, > > @@ -1867,6 +1871,12 @@ void intel_infoframe_init(struct intel_digital_port > > *intel_dig_port); > > u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder, > > const struct intel_crtc_state *crtc_state); > > u32 intel_hdmi_infoframe_enable(unsigned int type); > > +void intel_hdmi_read_gcp_infoframe(struct intel_encoder *encoder, > > + struct intel_crtc_state *crtc_state); > > +bool intel_read_infoframe(struct intel_encoder *encoder, > > + const struct intel_crtc_state *crtc_state, > > + enum hdmi_infoframe_type type, > > + union hdmi_infoframe *frame); > > > > > > /* intel_lvds.c */ > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > > b/drivers/gpu/drm/i915/intel_hdmi.c > > index 491001fc0fad..27cb6ec32e94 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -203,6 +203,31 @@ static void g4x_write_infoframe(struct intel_encoder > > *encoder, > > POSTING_READ(VIDEO_DIP_CTL); > > } > > > > +static ssize_t g4x_read_infoframe(struct intel_encoder *encoder, > > + const struct intel_crtc_state *crtc_state, > > + unsigned int type, > > + void *frame, ssize_t len) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + u32 val, *data = frame; > > + int i; > > + > > + val = I915_READ(VIDEO_DIP_CTL); > > + > > + if ((val & g4x_infoframe_enable(type)) == 0) > > + return 0; > > + > > + val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */ > > + val |= g4x_infoframe_index(type); > > + > > + I915_WRITE(VIDEO_DIP_CTL, val); > > + > > + for (i = 0; i < len; i += 4) > > + *data++ = I915_READ(VIDEO_DIP_DATA); > > + > > + return len; > > +} > > + > > static u32 g4x_infoframes_enabled(struct intel_encoder *encoder, > > const struct intel_crtc_state *pipe_config) > > { > > @@ -258,6 +283,32 @@ static void ibx_write_infoframe(struct intel_encoder > > *encoder, > > POSTING_READ(reg); > > } > > > > +static ssize_t ibx_read_infoframe(str
Re: [Intel-gfx] [PATCH 16/18] drm/i915/sdvo: Read out HDMI infoframes
On Mon, Sep 24, 2018 at 06:10:14PM +0200, Daniel Vetter wrote: > On Thu, Sep 20, 2018 at 09:51:43PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Read the HDMI infoframes from the hbuf and unpack them into > > the crtc state. > > > > Well, actually just AVI infoframe for now but let's write the > > infoframe readout code in a more generic fashion in case we > > expand this later. > > > > Signed-off-by: Ville Syrjälä > > Hm, caring about sdvo seems a bit overkill. And afaik we don't have any > sdvo (much less hdmi) in CI. I'm leaning towards just adding a > PIPE_CONFIG_QUIRK_INFOFRAMES for sdvo, and short-circuiting the checks if > that's set. Except if you can somehow convince CI folks to add an sdvo > hdmi card to CI :-) Unfortunately I only have one SDVO HDMI device and it has the chip straight on the motherboard. I can't give mine up for ci :) I guess we could try to find another one of those as that model doesn't even seem super rare. Just the annoying usual problem of getting one from somewhere approved. I think having to maintain a quirk is ~500% more annoying than adding the readout code. > > > --- > > drivers/gpu/drm/i915/intel_sdvo.c | 92 > > +-- > > 1 file changed, 89 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > > b/drivers/gpu/drm/i915/intel_sdvo.c > > index d8c78aebaf01..4d787c86df6d 100644 > > --- a/drivers/gpu/drm/i915/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > > @@ -981,6 +981,58 @@ static bool intel_sdvo_write_infoframe(struct > > intel_sdvo *intel_sdvo, > > &tx_rate, 1); > > } > > > > +static ssize_t intel_sdvo_read_infoframe(struct intel_sdvo *intel_sdvo, > > +unsigned int if_index, > > +u8 *data, unsigned int length) > > +{ > > + u8 set_buf_index[2] = { if_index, 0 }; > > + u8 hbuf_size, tx_rate, av_split; > > + int i; > > + > > + if (!intel_sdvo_get_value(intel_sdvo, > > + SDVO_CMD_GET_HBUF_AV_SPLIT, > > + &av_split, 1)) > > + return -ENXIO; > > + > > + if (av_split < if_index) > > + return 0; > > + > > + if (!intel_sdvo_get_value(intel_sdvo, > > + SDVO_CMD_GET_HBUF_TXRATE, > > + &tx_rate, 1)) > > + return -ENXIO; > > + > > + if (tx_rate == SDVO_HBUF_TX_DISABLED) > > + return 0; > > + > > + if (!intel_sdvo_set_value(intel_sdvo, > > + SDVO_CMD_SET_HBUF_INDEX, > > + set_buf_index, 2)) > > + return -ENXIO; > > + > > + if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HBUF_INFO, > > + &hbuf_size, 1)) > > + return -ENXIO; > > + > > + /* Buffer size is 0 based, hooray! */ > > + hbuf_size++; > > + > > + DRM_DEBUG_KMS("reading sdvo hbuf: %i, hbuf_size %i, hbuf_size: %i\n", > > + if_index, length, hbuf_size); > > + > > + hbuf_size = min_t(unsigned int, length, hbuf_size); > > + > > + for (i = 0; i < hbuf_size; i += 8) { > > + if (!intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_GET_HBUF_DATA, > > NULL, 0)) > > + return -ENXIO; > > + if (!intel_sdvo_read_response(intel_sdvo, &data[i], > > + min_t(unsigned int, 8, hbuf_size > > - i))) > > + return -ENXIO; > > + } > > + > > + return hbuf_size; > > +} > > + > > static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo, > > struct intel_crtc_state > > *crtc_state, > > struct drm_connector_state > > *conn_state) > > @@ -1039,6 +1091,37 @@ static bool intel_sdvo_set_avi_infoframe(struct > > intel_sdvo *intel_sdvo, > > sdvo_data, sizeof(sdvo_data)); > > } > > > > +static bool intel_sdvo_get_avi_infoframe(struct intel_sdvo *intel_sdvo, > > +struct intel_crtc_state *crtc_state) > > +{ > > + u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)]; > > + union hdmi_infoframe *frame = &crtc_state->infoframes.avi; > > + ssize_t len; > > + int ret; > > + > > + if (!crtc_state->has_hdmi_sink) > > + return true; > > + > > + len = intel_sdvo_read_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF, > > + sdvo_data, sizeof(sdvo_data)); > > + if (len < 0) > > + return false; > > + else if (len == 0) > > + return true; > > + > > + crtc_state->infoframes.enable |= > > + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI); > > + > > + ret = hdmi_infoframe_unpack(frame, sdvo_data, sizeof(sdvo_data)); > > + if (ret) > > + return false; > > + > > + if (frame->any.type != HDMI_INFOFRAME_TYPE_AVI)
[Bug 107428] Flickering artifacts in OpenRA with Vega 56
https://bugs.freedesktop.org/show_bug.cgi?id=107428 Isaac Curtis changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG --- Comment #4 from Isaac Curtis --- The bug has been identified. It can be seen on Lines 40-53 of https://github.com/OpenRA/OpenRA/blob/bfcbe8c0043bfadd1b38c4987f133e29aa54275c/glsl/shp.frag Basically, it was a floating point comparison bug. It checked for samplerIndex a float that takes int values, against 1.0, 2.0, etc. However, sometimes 1 is less than 1 due to floating point issues. Changing samplerIndex < 1.0 to samplerIndex < 0.5 fixes the issue. It turns out that AMDGPU drivers seem to be the only implementation that experience this particular issue. This case is resolved. I don't see anything to fix in the drivers here. -- 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 v2 0/3] A DRM API for adaptive sync and variable refresh rate support
These patches are part of a proposed new interface for supporting variable refresh rate via DRM properties. === Changes from v1 === For drm: * The variable_refresh_capable property is now flagged as DRM_MODE_PROP_IMMUTABLE For drm/gpu/amd/display: * Patches no longer pull in IOCTL/FreeSync refactoring code * FreeSync enable/disable behavior has been modified to reflect changes in userspace behavior from xf86-video-amdgpu and mesa === Adaptive sync and variable refresh rate === Adaptive sync is part of the DisplayPort spec and allows for graphics adapters to drive displays with varying frame timings. Variable refresh rate (VRR) is essentially the same, but defined for HDMI. === Use cases for variable refresh rate === Variable frame (flip) timings don't align well with fixed refresh rate displays. This results in stuttering, tearing and/or input lag. By adjusting the display refresh rate dynamically these issues can be reduced or eliminated. However, not all content is suitable for dynamic refresh adaptation. Content that is flipped infrequently or at random intervals tends to fair poorly. Multiple clients trying to flip under the same screen can similarly interfere with prediction. Userland needs a way to let the driver know when the content on the screen is suitable for variable refresh rate and if the user wishes to have the feature enabled. === DRM API to support variable refresh rates === This patch introduces a new API via atomic properties on the DRM connector and CRTC. The connector has two new optional properties: * bool variable_refresh_capable - set by the driver if the hardware is capable of supporting variable refresh tech * bool variable_refresh_enabled - set by the user to enable variable refresh adjustment over the connector The CRTC has one additional default property: * bool variable_refresh - a content hint to the driver specifying that the CRTC contents are suitable for variable refresh adjustment == Overview for DRM driver developers === Driver developers can attach the optional connector properties via drm_connector_attach_variable_refresh_properties on connectors that support variable refresh (typically DP or HDMI). The variable_refresh_capable property should be managed as the output on the connector changes. The property is read only from userspace. The variable_refresh_enabled property is intended to be a property controlled by userland as a global on/off switch for variable refresh technology. It should be checked before enabling variable refresh rate. === Overview for Userland developers == The variable_refresh property on the CRTC should be set to true when the CRTCs are suitable for variable refresh rate. In practice this is probably an application like a game - a single window that covers the whole CRTC surface and is the only client issuing flips. To demonstrate the suitability of the API for variable refresh and dynamic adaptation there are additional patches using this API that implement adaptive variable refresh across kernel and userland projects: * DRM (dri-devel) * amdgpu DRM kernel driver (amd-gfx) * xf86-video-amdgpu (amd-gfx) * mesa (mesa-dev) These patches enable adaptive variable refresh on X for AMD hardware provided that the user sets the variable_refresh_enabled property to true on supported connectors (ie. using xrandr --set-prop). The patches have been tested as working on upstream userland with the GNOME desktop environment under a single monitor setup. They also work on KDE in single monitor setup if the compositor is disabled. The patches require that the application window can issue screen flips via the Present extension to xf86-video-amdgpu. Due to Present extension limitations some desktop environments and multi-monitor setups are currently not compatible. Full implementation details for these changes can be reviewed in their respective mailing lists. === Previous discussions === These patches are based upon feedback from patches and feedback from two previous threads on the subject which are linked below for reference: https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.html Nicholas Kazlauskas Nicholas Kazlauskas (3): drm: Add variable refresh rate properties to connector drm: Add variable refresh property to DRM CRTC drm/amd/display: Set FreeSync state using DRM VRR properties .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 232 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 6 +- drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_atomic_uapi.c | 12 + drivers/gpu/drm/drm_connector.c | 35 +++ drivers/gpu/drm/drm_crtc.c| 2 + drivers/gpu/drm/drm_mode_config.c | 6 + include/drm/drm_connector.h
[PATCH v2 1/3] drm: Add variable refresh rate properties to connector
Modern display hardware is capable of supporting variable refresh rates and adaptive sync technologies. The properties for querying and controlling these features should be exposed on the DRM connector. This patch introduces two new properties for variable refresh rate support: - variable_refresh_capable - variable_refresh_enabled These are optional properties that can be added to a DRM connector dynamically by using drm_connector_attach_variable_refresh_properties. DRM drivers should set variable_refresh_capable as applicable for their hardware. The property variable_refresh_enabled is a userspace controlled option. Signed-off-by: Nicholas Kazlauskas --- drivers/gpu/drm/drm_atomic_uapi.c | 6 ++ drivers/gpu/drm/drm_connector.c | 35 +++ include/drm/drm_connector.h | 27 3 files changed, 68 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index d5b7f315098c..0bb27a24a55c 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -723,6 +723,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, state->content_type = val; } else if (property == connector->scaling_mode_property) { state->scaling_mode = val; + } else if (property == connector->variable_refresh_enabled_property) { + state->variable_refresh_enabled = val; } else if (property == connector->content_protection_property) { if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) { DRM_DEBUG_KMS("only drivers can set CP Enabled\n"); @@ -797,6 +799,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector, *val = state->content_type; } else if (property == connector->scaling_mode_property) { *val = state->scaling_mode; + } else if (property == connector->variable_refresh_capable_property) { + *val = state->variable_refresh_capable; + } else if (property == connector->variable_refresh_enabled_property) { + *val = state->variable_refresh_enabled; } else if (property == connector->content_protection_property) { *val = state->content_protection; } else if (property == config->writeback_fb_id_property) { diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 1e40e5decbe9..fc1732639bd3 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1254,6 +1254,41 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev) } EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); +/** + * drm_connector_attach_variable_refresh_properties - creates and attaches + * properties for connectors that support adaptive refresh + * @connector: connector to create adaptive refresh properties on + */ +int drm_connector_attach_variable_refresh_properties( + struct drm_connector *connector) +{ + struct drm_device *dev = connector->dev; + struct drm_property *prop; + + if (!connector->variable_refresh_capable_property) { + prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE, + "variable_refresh_capable"); + if (!prop) + return -ENOMEM; + + connector->variable_refresh_capable_property = prop; + drm_object_attach_property(&connector->base, prop, 0); + } + + if (!connector->variable_refresh_enabled_property) { + prop = drm_property_create_bool(dev, 0, + "variable_refresh_enabled"); + if (!prop) + return -ENOMEM; + + connector->variable_refresh_enabled_property = prop; + drm_object_attach_property(&connector->base, prop, 0); + } + + return 0; +} +EXPORT_SYMBOL(drm_connector_attach_variable_refresh_properties); + /** * drm_connector_attach_scaling_mode_property - attach atomic scaling mode property * @connector: connector to attach scaling mode property on. diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 91a877fa00cb..e2c26842bd50 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -449,6 +449,18 @@ struct drm_connector_state { */ unsigned int content_protection; + /** +* @variable_refresh_enabled: Connector property used to check +* if variable refresh is supported on the device. +*/ + bool variable_refresh_capable; + + /** +* @variable_refresh_enabled: Connector property used to check +* if variable refresh is enabled. +*/ + bool variable_refresh_enabled; + /** * @writeback_job: Writeback job for writeback connectors * @@ -910,6 +922,19 @@ struct drm_connector { */
[PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC
Variable refresh rate algorithms have typically been enabled only when the display is covered by a single source of content. This patch introduces a new default CRTC property that helps hint to the driver when the CRTC composition is suitable for variable refresh rate algorithms. Userspace can set this property dynamically as the composition changes. Whether the variable refresh rate algorithms are active will still depend on the CRTC being suitable and the connector being capable and enabled by the user for variable refresh rate support. It is worth noting that while the property is atomic it isn't filtered from legacy userspace queries. This allows for Xorg userspace drivers to implement support in non-atomic setups. Signed-off-by: Nicholas Kazlauskas --- drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_atomic_uapi.c | 6 ++ drivers/gpu/drm/drm_crtc.c | 2 ++ drivers/gpu/drm/drm_mode_config.c | 6 ++ include/drm/drm_crtc.h | 13 + include/drm/drm_mode_config.h | 8 6 files changed, 36 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 3cf1aa132778..b768d397a811 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3473,6 +3473,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, state->planes_changed = false; state->connectors_changed = false; state->color_mgmt_changed = false; + state->variable_refresh_changed = false; state->zpos_changed = false; state->commit = NULL; state->event = NULL; diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 0bb27a24a55c..32a4cf8a01c3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -433,6 +433,10 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_set_mode_prop_for_crtc(state, mode); drm_property_blob_put(mode); return ret; + } else if (property == config->prop_variable_refresh) { + if (state->variable_refresh != val) + state->variable_refresh_changed = true; + state->variable_refresh = val; } else if (property == config->degamma_lut_property) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->degamma_lut, @@ -491,6 +495,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = state->active; else if (property == config->prop_mode_id) *val = (state->mode_blob) ? state->mode_blob->base.id : 0; + else if (property == config->prop_variable_refresh) + *val = state->variable_refresh; else if (property == config->degamma_lut_property) *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0; else if (property == config->ctm_property) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 5f488aa80bcd..ca33d6fb90ac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -340,6 +340,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, drm_object_attach_property(&crtc->base, config->prop_mode_id, 0); drm_object_attach_property(&crtc->base, config->prop_out_fence_ptr, 0); + drm_object_attach_property(&crtc->base, + config->prop_variable_refresh, 0); } return 0; diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index ee80788f2c40..1287c161d65d 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -310,6 +310,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = prop; + prop = drm_property_create_bool(dev, 0, + "VARIABLE_REFRESH"); + if (!prop) + return -ENOMEM; + dev->mode_config.prop_variable_refresh = prop; + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, "DEGAMMA_LUT", 0); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b21437bc95bf..32b77f18ce6d 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -168,6 +168,12 @@ struct drm_crtc_state { * drivers to steer the atomic commit control flow. */ bool color_mgmt_changed : 1; + /** +* @variable_refresh_changed: Variable refresh support has changed +* on the CRTC. Used by the atomic helpers and drivers to steer the +* atomic commit control flow. +*/ + bool variable_refresh_changed : 1; /*
[PATCH v2 3/3] drm/amd/display: Set FreeSync state using DRM VRR properties
The AMDGPU specific FreeSync properties and IOCTLs are dropped from amdgpu_dm in favor of the DRM variable refresh properties. The AMDGPU connector properties freesync_capable and freesync_enabled are mostly direct mappings to the DRM variable_refresh_capable and variable_refresh_enabled. The AMDGPU CRTC freesync_enabled property requires special care to handle correctly. The DRM variable_refresh property is a content hint to the driver which is independent from actual hardware variable refresh capability and user configuration. To handle changes with this property it was easier to drop the forced modeset on variable refresh change. This patch now performs the required stream updates when planes are attached *or* flipped. This is done for a few reasons: (1) VRR stream updates can be done in the fast update path (2) amdgpu_dm_atomic_check would have had to been hacked apart to check desired variable refresh state and capability before the CRTC disable pass. (3) Performing VRR stream updates on-flip is needed for enabling BTR support. FreeSync state on the stream is now tracked and compared to the previous packet sent to the hardware. It's worth noting that the current implementation treats variable_refresh_enabled as a strict requirement for sending the VRR enable *or* disable packet. Signed-off-by: Nicholas Kazlauskas --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 232 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 6 +- 2 files changed, 121 insertions(+), 117 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 6c849e055038..0fddc2e66f49 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1683,72 +1683,6 @@ static void dm_bandwidth_update(struct amdgpu_device *adev) /* TODO: implement later */ } -static int amdgpu_notify_freesync(struct drm_device *dev, void *data, - struct drm_file *filp) -{ - struct drm_atomic_state *state; - struct drm_modeset_acquire_ctx ctx; - struct drm_crtc *crtc; - struct drm_connector *connector; - struct drm_connector_state *old_con_state, *new_con_state; - int ret = 0; - uint8_t i; - bool enable = false; - - drm_modeset_acquire_init(&ctx, 0); - - state = drm_atomic_state_alloc(dev); - if (!state) { - ret = -ENOMEM; - goto out; - } - state->acquire_ctx = &ctx; - -retry: - drm_for_each_crtc(crtc, dev) { - ret = drm_atomic_add_affected_connectors(state, crtc); - if (ret) - goto fail; - - /* TODO rework amdgpu_dm_commit_planes so we don't need this */ - ret = drm_atomic_add_affected_planes(state, crtc); - if (ret) - goto fail; - } - - for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { - struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state); - struct drm_crtc_state *new_crtc_state; - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(dm_new_con_state->base.crtc); - struct dm_crtc_state *dm_new_crtc_state; - - if (!acrtc) { - ASSERT(0); - continue; - } - - new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base); - dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); - - dm_new_crtc_state->freesync_enabled = enable; - } - - ret = drm_atomic_commit(state); - -fail: - if (ret == -EDEADLK) { - drm_atomic_state_clear(state); - drm_modeset_backoff(&ctx); - goto retry; - } - - drm_atomic_state_put(state); - -out: - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - return ret; -} static const struct amdgpu_display_funcs dm_display_funcs = { .bandwidth_update = dm_bandwidth_update, /* called unconditionally */ @@ -1762,7 +1696,6 @@ static const struct amdgpu_display_funcs dm_display_funcs = { dm_crtc_get_scanoutpos,/* called unconditionally */ .add_encoder = NULL, /* VBIOS parsing. DAL does it. */ .add_connector = NULL, /* VBIOS parsing. DAL does it. */ - .notify_freesync = amdgpu_notify_freesync, }; @@ -2645,7 +2578,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector, update_stream_signal(stream); - if (dm_state && dm_state->freesync_capable) + if (dm_state && dm_state->base.variable_refresh_capable) stream->ignore_msa_timing_param = true; finish: if (sink && sink->sink_signal == SIGNAL_TYPE_VIRTUAL) @@ -2713,9 +2646,10 @@ dm_crtc_duplicate_state(struct
Re: [PATCH v2 1/3] drm: Add variable refresh rate properties to connector
On Mon, Sep 24, 2018 at 02:15:35PM -0400, Nicholas Kazlauskas wrote: > Modern display hardware is capable of supporting variable refresh rates > and adaptive sync technologies. The properties for querying and > controlling these features should be exposed on the DRM connector. > > This patch introduces two new properties for variable refresh rate > support: > > - variable_refresh_capable > - variable_refresh_enabled > > These are optional properties that can be added to a DRM connector > dynamically by using drm_connector_attach_variable_refresh_properties. > > DRM drivers should set variable_refresh_capable as applicable for > their hardware. The property variable_refresh_enabled is a userspace > controlled option. > > Signed-off-by: Nicholas Kazlauskas > --- > drivers/gpu/drm/drm_atomic_uapi.c | 6 ++ > drivers/gpu/drm/drm_connector.c | 35 +++ > include/drm/drm_connector.h | 27 > 3 files changed, 68 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index d5b7f315098c..0bb27a24a55c 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -723,6 +723,8 @@ static int drm_atomic_connector_set_property(struct > drm_connector *connector, > state->content_type = val; > } else if (property == connector->scaling_mode_property) { > state->scaling_mode = val; > + } else if (property == connector->variable_refresh_enabled_property) { > + state->variable_refresh_enabled = val; > } else if (property == connector->content_protection_property) { > if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) { > DRM_DEBUG_KMS("only drivers can set CP Enabled\n"); > @@ -797,6 +799,10 @@ drm_atomic_connector_get_property(struct drm_connector > *connector, > *val = state->content_type; > } else if (property == connector->scaling_mode_property) { > *val = state->scaling_mode; > + } else if (property == connector->variable_refresh_capable_property) { > + *val = state->variable_refresh_capable; Immutable props don't take this path. See __drm_object_property_get_value(). > + } else if (property == connector->variable_refresh_enabled_property) { > + *val = state->variable_refresh_enabled; > } else if (property == connector->content_protection_property) { > *val = state->content_protection; > } else if (property == config->writeback_fb_id_property) { > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 1e40e5decbe9..fc1732639bd3 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1254,6 +1254,41 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > +/** > + * drm_connector_attach_variable_refresh_properties - creates and attaches > + * properties for connectors that support adaptive refresh > + * @connector: connector to create adaptive refresh properties on > + */ > +int drm_connector_attach_variable_refresh_properties( > + struct drm_connector *connector) > +{ > + struct drm_device *dev = connector->dev; > + struct drm_property *prop; > + > + if (!connector->variable_refresh_capable_property) { > + prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE, > + "variable_refresh_capable"); > + if (!prop) > + return -ENOMEM; > + > + connector->variable_refresh_capable_property = prop; > + drm_object_attach_property(&connector->base, prop, 0); > + } > + > + if (!connector->variable_refresh_enabled_property) { > + prop = drm_property_create_bool(dev, 0, > + "variable_refresh_enabled"); > + if (!prop) > + return -ENOMEM; > + > + connector->variable_refresh_enabled_property = prop; > + drm_object_attach_property(&connector->base, prop, 0); > + } > + > + return 0; > +} > +EXPORT_SYMBOL(drm_connector_attach_variable_refresh_properties); > + > /** > * drm_connector_attach_scaling_mode_property - attach atomic scaling mode > property > * @connector: connector to attach scaling mode property on. > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 91a877fa00cb..e2c26842bd50 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -449,6 +449,18 @@ struct drm_connector_state { >*/ > unsigned int content_protection; > > + /** > + * @variable_refresh_enabled: Connector property used to check > + * if variable refresh is supported on the device. > + */ > + bool variable_refresh_capable; > + > + /** > + * @variable_refresh_enabled
Re: [PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC
On Mon, Sep 24, 2018 at 02:15:36PM -0400, Nicholas Kazlauskas wrote: > Variable refresh rate algorithms have typically been enabled only > when the display is covered by a single source of content. > > This patch introduces a new default CRTC property that helps > hint to the driver when the CRTC composition is suitable for variable > refresh rate algorithms. Userspace can set this property dynamically > as the composition changes. > > Whether the variable refresh rate algorithms are active will still > depend on the CRTC being suitable and the connector being capable > and enabled by the user for variable refresh rate support. > > It is worth noting that while the property is atomic it isn't filtered > from legacy userspace queries. This allows for Xorg userspace drivers > to implement support in non-atomic setups. > > Signed-off-by: Nicholas Kazlauskas > --- > drivers/gpu/drm/drm_atomic_helper.c | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 6 ++ > drivers/gpu/drm/drm_crtc.c | 2 ++ > drivers/gpu/drm/drm_mode_config.c | 6 ++ > include/drm/drm_crtc.h | 13 + > include/drm/drm_mode_config.h | 8 > 6 files changed, 36 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 3cf1aa132778..b768d397a811 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -3473,6 +3473,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct > drm_crtc *crtc, > state->planes_changed = false; > state->connectors_changed = false; > state->color_mgmt_changed = false; > + state->variable_refresh_changed = false; Another bool just to check if one bool changed seems a bit excessive. Is there a reason you can't directly check if the other bool changed? Actually I don't understand why this per-crtc thing is being added at all. You already have the prop on the connector. Why do we want to make life more difficult by requiring the thing to be set on both the crtc and connector? > state->zpos_changed = false; > state->commit = NULL; > state->event = NULL; > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index 0bb27a24a55c..32a4cf8a01c3 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -433,6 +433,10 @@ static int drm_atomic_crtc_set_property(struct drm_crtc > *crtc, > ret = drm_atomic_set_mode_prop_for_crtc(state, mode); > drm_property_blob_put(mode); > return ret; > + } else if (property == config->prop_variable_refresh) { > + if (state->variable_refresh != val) > + state->variable_refresh_changed = true; > + state->variable_refresh = val; > } else if (property == config->degamma_lut_property) { > ret = drm_atomic_replace_property_blob_from_id(dev, > &state->degamma_lut, > @@ -491,6 +495,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, > *val = state->active; > else if (property == config->prop_mode_id) > *val = (state->mode_blob) ? state->mode_blob->base.id : 0; > + else if (property == config->prop_variable_refresh) > + *val = state->variable_refresh; > else if (property == config->degamma_lut_property) > *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0; > else if (property == config->ctm_property) > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 5f488aa80bcd..ca33d6fb90ac 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -340,6 +340,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, > struct drm_crtc *crtc, > drm_object_attach_property(&crtc->base, config->prop_mode_id, > 0); > drm_object_attach_property(&crtc->base, > config->prop_out_fence_ptr, 0); > + drm_object_attach_property(&crtc->base, > +config->prop_variable_refresh, 0); > } > > return 0; > diff --git a/drivers/gpu/drm/drm_mode_config.c > b/drivers/gpu/drm/drm_mode_config.c > index ee80788f2c40..1287c161d65d 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -310,6 +310,12 @@ static int drm_mode_create_standard_properties(struct > drm_device *dev) > return -ENOMEM; > dev->mode_config.prop_mode_id = prop; > > + prop = drm_property_create_bool(dev, 0, > + "VARIABLE_REFRESH"); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.prop_variable_refresh = prop; > + > prop = drm_property_create(dev, > DRM_MODE_PROP_BLOB, > "DEGAMMA_LUT", 0); > diff --git a/include/drm/
Re: [PATCH v2 03/16] dt-bindings: display: renesas: lvds: Add EXTAL and DU_DOTCLKIN clocks
Hi Laurent, Thank you for the patch, On 14/09/18 10:10, Laurent Pinchart wrote: > On the D3 and E3 SoCs, the LVDS encoder can derive its internal pixel > clock from an externally supplied clock, either through the EXTAL pin or > through one of the DU_DOTCLKINx pins. Add corresponding clocks to the DT > bindings. > > To retain backward compatibility with DT that don't specify the > clock-names property, the functional clock must always be specified > first, and the clock-names property is optional when only the functional > clock is specified. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Jacopo Mondi > --- > Changes since v1: > > - Clarified wording > --- > .../devicetree/bindings/display/bridge/renesas,lvds.txt | 12 > +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > index 13af7e2ac7e8..3aeb0ec06fd0 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > @@ -19,7 +19,17 @@ Required properties: >- "renesas,r8a77995-lvds" for R8A77995 (R-Car D3) compatible LVDS encoders > > - reg: Base address and length for the memory-mapped registers > -- clocks: A phandle + clock-specifier pair for the functional clock > +- clocks: A list of phandles + clock-specifier pairs, one for each entry in > + the clock-names property. > +- clock-names: Name of the clocks. This property is model-dependent. > + - The functional clock, which mandatory for all models, shall be listed > +first, and shall be named "fck". > + - On R8A77990 and R8A77995, the LVDS encoder can use the EXTAL or > +DU_DOTCLKINx clocks. Those clocks are optional. When supplied they must > be > +named "extal" and "dclkin.x" respectively, with "x" being the DU_DOTCLKIN > +numerical index. > + - When the clocks property only contains the functional clock, the > +clock-names property may be omitted. This all reads well to me. Reviewed-by: Kieran Bingham > - resets: A phandle + reset specifier for the module reset > > Required nodes: > -- Regards -- Kieran ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC
On 09/24/2018 02:38 PM, Ville Syrjälä wrote: On Mon, Sep 24, 2018 at 02:15:36PM -0400, Nicholas Kazlauskas wrote: Variable refresh rate algorithms have typically been enabled only when the display is covered by a single source of content. This patch introduces a new default CRTC property that helps hint to the driver when the CRTC composition is suitable for variable refresh rate algorithms. Userspace can set this property dynamically as the composition changes. Whether the variable refresh rate algorithms are active will still depend on the CRTC being suitable and the connector being capable and enabled by the user for variable refresh rate support. It is worth noting that while the property is atomic it isn't filtered from legacy userspace queries. This allows for Xorg userspace drivers to implement support in non-atomic setups. Signed-off-by: Nicholas Kazlauskas --- drivers/gpu/drm/drm_atomic_helper.c | 1 + drivers/gpu/drm/drm_atomic_uapi.c | 6 ++ drivers/gpu/drm/drm_crtc.c | 2 ++ drivers/gpu/drm/drm_mode_config.c | 6 ++ include/drm/drm_crtc.h | 13 + include/drm/drm_mode_config.h | 8 6 files changed, 36 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 3cf1aa132778..b768d397a811 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3473,6 +3473,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, state->planes_changed = false; state->connectors_changed = false; state->color_mgmt_changed = false; + state->variable_refresh_changed = false; Another bool just to check if one bool changed seems a bit excessive. Is there a reason you can't directly check if the other bool changed? It's nice for an atomic driver to be able to check if the property has changed to steer control flow. The driver could just check if the old crtc variable refresh value isn't equal to the new one itself, but there's already precedent set to provide flags like these instead. It also lets the driver change it as needed during atomic commits. You'll see many drivers making use of the other flags like connectors_changed, mode_changed, etc. Actually I don't understand why this per-crtc thing is being added at all. You already have the prop on the connector. Why do we want to make life more difficult by requiring the thing to be set on both the crtc and connector? It doesn't make much sense without both. The user can globally enable or disable the variable_refresh_enabled on the connector. This is the user's preference. What they don't control is the variable_refresh - the content hint that userspace specifies when the CRTC contents are suitable for enabling variable refresh features (like adaptive sync). This is userspace's preference. When both preferences agree, only then will variable refresh features be enabled. The reasoning for the split is because not all content is suitable for variable refresh. Desktop environments, web browsers, etc only typically flip when needed - which will result in display flickering. The userspace integration as part of these patches demonstrates enabling variable_refresh on the CRTC selectively. state->zpos_changed = false; state->commit = NULL; state->event = NULL; diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 0bb27a24a55c..32a4cf8a01c3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -433,6 +433,10 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_set_mode_prop_for_crtc(state, mode); drm_property_blob_put(mode); return ret; + } else if (property == config->prop_variable_refresh) { + if (state->variable_refresh != val) + state->variable_refresh_changed = true; + state->variable_refresh = val; } else if (property == config->degamma_lut_property) { ret = drm_atomic_replace_property_blob_from_id(dev, &state->degamma_lut, @@ -491,6 +495,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = state->active; else if (property == config->prop_mode_id) *val = (state->mode_blob) ? state->mode_blob->base.id : 0; + else if (property == config->prop_variable_refresh) + *val = state->variable_refresh; else if (property == config->degamma_lut_property) *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0; else if (property == config->ctm_property) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 5f488aa80bcd..ca33d6fb90ac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -340,6
Re: [PATCH v2 6/7] drm: meson: use xxxsetbits32
Hi Corentin, On 24/09/2018 21:04, Corentin Labbe wrote: > This patch convert meson DRM driver to use all xxxsetbits32 functions. > > Signed-off-by: Corentin Labbe > --- > drivers/gpu/drm/meson/meson_crtc.c | 14 --- > drivers/gpu/drm/meson/meson_dw_hdmi.c | 33 + > drivers/gpu/drm/meson/meson_plane.c | 13 --- > drivers/gpu/drm/meson/meson_registers.h | 3 -- > drivers/gpu/drm/meson/meson_venc.c | 13 --- > drivers/gpu/drm/meson/meson_venc_cvbs.c | 4 +- > drivers/gpu/drm/meson/meson_viu.c | 65 > + > drivers/gpu/drm/meson/meson_vpp.c | 22 +-- > 8 files changed, 86 insertions(+), 81 deletions(-) > [...] Reviewed-by: Neil Armstrong Tested-by: Neil Armstrong ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
On 24/09/2018 21:04, Corentin Labbe wrote: > This patch convert meson stmmac glue driver to use all xxxsetbits32 functions. > > Signed-off-by: Corentin Labbe > --- > .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 56 > +- > 1 file changed, 22 insertions(+), 34 deletions(-) > [...] Reviewed-by: Neil Armstrong ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106671] Frequent lock ups for AMD RX 550 graphics card
https://bugs.freedesktop.org/show_bug.cgi?id=106671 --- Comment #21 from Alan W. Irwin --- Created attachment 141724 --> https://bugs.freedesktop.org/attachment.cgi?id=141724&action=edit tarball containing kern.log, syslog, and dmesg output -- 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 108049] [vega10] amdgpu fails to either wake up the GPU or while putting it to sleep
https://bugs.freedesktop.org/show_bug.cgi?id=108049 --- Comment #1 from Gediminas Jakutis --- Created attachment 141726 --> https://bugs.freedesktop.org/attachment.cgi?id=141726&action=edit log for dce_mi_allocate_dmif line:599 -- 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 108049] [vega10] amdgpu fails to either wake up the GPU or while putting it to sleep
https://bugs.freedesktop.org/show_bug.cgi?id=108049 Bug ID: 108049 Summary: [vega10] amdgpu fails to either wake up the GPU or while putting it to sleep Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: gedimi...@varciai.lt Created attachment 141725 --> https://bugs.freedesktop.org/attachment.cgi?id=141725&action=edit log for dce_mi_free_dmif line:636 If my machine with a Vega 64 idles for long enough for the GPU to be "put to sleep" to save power, it won't wake up. If given inputs, The monitors get wake up, but only show black, indicating that something is happening. Looking at kernel logs, this always starts with: [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 10us * 3500 tries - dce_mi_free_dmif line:636 (full log attached separately) While trying to "wake up" and/or if I try to switch to a different TTY, another error pops up for every attempt, but the opening line is now: [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 10us * 6000 tries - dce_mi_allocate_dmif line:599 (full log also attached separately) I experience this problem with every kernel version I tried from 4.16.x to and including 4.18.7 Haven't tried earlier versions and not sure which kernel to try to build in order to test the *latest* amdgpu code. Could test the latest amdgpu code if I could get pointed to it. As a workaround, I have a tiny video on mpv running on infinite loop in the background using an opengl output, to prevent the GPU from trying to sleep. -- 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