Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback
On Tue, May 2, 2017 at 11:29 AM, Jose Abreu wrote: > On 02-05-2017 09:48, Daniel Vetter wrote: >> On Wed, Apr 26, 2017 at 11:48:34AM +0100, Jose Abreu wrote: >>> Some crtc's may have restrictions in the mode they can display. In >>> this patch a new callback (crtc->mode_valid()) is introduced that >>> is called at the same stage of connector->mode_valid() callback. >>> >>> This shall be implemented if the crtc has some sort of restriction >>> so that we don't probe modes that will fail in the commit() stage. >>> For example: A given crtc may be responsible to set a clock value. >>> If the clock can not produce all the values for the available >>> modes then this callback can be used to restrict the number of >>> probbed modes to only the ones that can be displayed. >>> >>> If the crtc does not implement the callback then the behaviour will >>> remain the same. Also, for a given set of crtcs that can be bound to >>> the connector, if at least one can display the mode then the mode >>> will be probbed. >>> >>> Signed-off-by: Jose Abreu >>> Cc: Carlos Palminha >>> Cc: Alexey Brodkin >>> Cc: Ville Syrjälä >>> Cc: Daniel Vetter >>> Cc: Dave Airlie >> Not sure this is useful, since you still have to duplicate the exact same >> check into your ->mode_fixup hook. That seems to make things even more >> confusing. > > Yeah, in arcpgu I had to duplicate the code in ->atomic_check. > >> >> Also this doesn't update the various kerneldoc comments. For the existing >> hooks. Since this topic causes so much confusion, I don't think a >> half-solution will help, but has some good potential to make things worse. > > I only documented the callback in drm_modeset_helper_vtables.h. > > Despite all of this, I think it doesn't makes sense delivering > modes to userspace which can never be used. > > This is really annoying in arcpgu. Imagine: I try to use mpv to > play a video, the full set of modes from EDID were probed so if I > just start mpv it will pick the native mode of the TV instead of > the one that is supported, so mpv will fail to play. I know the > value of clock which will work (so I know what mode shall be > used), but a normal user which is not aware of the HW will have > to cycle through the list of modes and try them all until it hits > one that works. Its really boring. > > For the modes that user specifies manually there is nothing we > can do, but we should not trick users into thinking that a given > mode is supported when it will always fail at commit. Yes, you are supposed to filter these out in ->mode_valid. But my stance is that only adding a half-baked support for a new callback to the core isn't going to make life easier for drivers, it will just add to the confusion. There's already piles of docs for both @mode_valid and @mode_fixup hooks explaining this, I don't want to make the documentation even more complex. And half-baked crtc checking is _much_ easier to implement in the driver directly (e.g. i915 checks for crtc constraints since forever, as do the other big x86 drivers). So all taken together, if we add a ->mode_valid to crtcs, then imo we should do it right and actually make life easier for drivers. A good proof would be if your patch would allow us to drop a lot of the lenghty language from the @mode_valid hooks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback
On Tue, May 2, 2017 at 11:29 AM, Jose Abreu wrote: > On 02-05-2017 09:48, Daniel Vetter wrote: >> On Wed, Apr 26, 2017 at 11:48:34AM +0100, Jose Abreu wrote: >>> Some crtc's may have restrictions in the mode they can display. In >>> this patch a new callback (crtc->mode_valid()) is introduced that >>> is called at the same stage of connector->mode_valid() callback. >>> >>> This shall be implemented if the crtc has some sort of restriction >>> so that we don't probe modes that will fail in the commit() stage. >>> For example: A given crtc may be responsible to set a clock value. >>> If the clock can not produce all the values for the available >>> modes then this callback can be used to restrict the number of >>> probbed modes to only the ones that can be displayed. >>> >>> If the crtc does not implement the callback then the behaviour will >>> remain the same. Also, for a given set of crtcs that can be bound to >>> the connector, if at least one can display the mode then the mode >>> will be probbed. >>> >>> Signed-off-by: Jose Abreu >>> Cc: Carlos Palminha >>> Cc: Alexey Brodkin >>> Cc: Ville Syrjälä >>> Cc: Daniel Vetter >>> Cc: Dave Airlie >> Not sure this is useful, since you still have to duplicate the exact same >> check into your ->mode_fixup hook. That seems to make things even more >> confusing. > > Yeah, in arcpgu I had to duplicate the code in ->atomic_check. > >> >> Also this doesn't update the various kerneldoc comments. For the existing >> hooks. Since this topic causes so much confusion, I don't think a >> half-solution will help, but has some good potential to make things worse. > > I only documented the callback in drm_modeset_helper_vtables.h. > > Despite all of this, I think it doesn't makes sense delivering > modes to userspace which can never be used. > > This is really annoying in arcpgu. Imagine: I try to use mpv to > play a video, the full set of modes from EDID were probed so if I > just start mpv it will pick the native mode of the TV instead of > the one that is supported, so mpv will fail to play. I know the > value of clock which will work (so I know what mode shall be > used), but a normal user which is not aware of the HW will have > to cycle through the list of modes and try them all until it hits > one that works. Its really boring. > > For the modes that user specifies manually there is nothing we > can do, but we should not trick users into thinking that a given > mode is supported when it will always fail at commit. Yes, you are supposed to filter these out in ->mode_valid. But my stance is that only adding a half-baked support for a new callback to the core isn't going to make life easier for drivers, it will just add to the confusion. There's already piles of docs for both @mode_valid and @mode_fixup hooks explaining this, I don't want to make the documentation even more complex. And half-baked crtc checking is _much_ easier to implement in the driver directly (e.g. i915 checks for crtc constraints since forever, as do the other big x86 drivers). So all taken together, if we add a ->mode_valid to crtcs, then imo we should do it right and actually make life easier for drivers. A good proof would be if your patch would allow us to drop a lot of the lenghty language from the @mode_valid hooks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/i915: Add format modifiers for Intel
This was based on a patch originally by Kristian. It has been modified
pretty heavily to use the new callbacks from the previous patch.
v2:
- Add LINEAR and Yf modifiers to list (Ville)
- Combine i8xx and i965 into one list of formats (Ville)
- Allow 1010102 formats for Y/Yf tiled (Ville)
v3:
- Handle cursor formats (Ville)
- Put handling for LINEAR in the mod_support functions (Ville)
v4:
- List each modifier explicitly in supported modifiers (Ville)
- Handle the CURSOR plane (Ville)
v5:
- Split out cursor and sprite handling (Ville)
Cc: Ville Syrjälä
Cc: Kristian H. Kristensen
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/intel_display.c | 131 +--
drivers/gpu/drm/i915/intel_sprite.c | 76 +++-
2 files changed, 201 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d52bd05a017d..18aac538d978 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -72,6 +72,12 @@ static const uint32_t i965_primary_formats[] = {
DRM_FORMAT_XBGR2101010,
};
+static const uint64_t i9xx_format_modifiers[] = {
+ I915_FORMAT_MOD_X_TILED,
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID
+};
+
static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_C8,
DRM_FORMAT_RGB565,
@@ -87,6 +93,14 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_VYUY,
};
+static const uint64_t skl_format_modifiers[] = {
+ I915_FORMAT_MOD_Yf_TILED,
+ I915_FORMAT_MOD_Y_TILED,
+ I915_FORMAT_MOD_X_TILED,
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID
+};
+
/* Cursor formats */
static const uint32_t intel_cursor_formats[] = {
DRM_FORMAT_ARGB,
@@ -13381,6 +13395,103 @@ void intel_plane_destroy(struct drm_plane *plane)
kfree(to_intel_plane(plane));
}
+static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
+{
+ switch (format) {
+ case DRM_FORMAT_C8:
+ case DRM_FORMAT_RGB565:
+ case DRM_FORMAT_XRGB1555:
+ case DRM_FORMAT_XRGB:
+ return modifier == DRM_FORMAT_MOD_LINEAR ||
+ modifier == I915_FORMAT_MOD_X_TILED;
+ default:
+ return false;
+ }
+}
+
+static bool i965_mod_supported(uint32_t format, uint64_t modifier)
+{
+ switch (format) {
+ case DRM_FORMAT_C8:
+ case DRM_FORMAT_RGB565:
+ case DRM_FORMAT_XRGB:
+ case DRM_FORMAT_XBGR:
+ case DRM_FORMAT_XRGB2101010:
+ case DRM_FORMAT_XBGR2101010:
+ return modifier == DRM_FORMAT_MOD_LINEAR ||
+ modifier == I915_FORMAT_MOD_X_TILED;
+ default:
+ return false;
+ }
+}
+
+static bool skl_mod_supported(uint32_t format, uint64_t modifier)
+{
+ switch (format) {
+ case DRM_FORMAT_C8:
+ switch (modifier) {
+ case DRM_FORMAT_MOD_LINEAR:
+ case I915_FORMAT_MOD_X_TILED:
+ case I915_FORMAT_MOD_Y_TILED:
+ return true;
+ default:
+ return false;
+ }
+ case DRM_FORMAT_RGB565:
+ case DRM_FORMAT_XRGB:
+ case DRM_FORMAT_XBGR:
+ case DRM_FORMAT_ARGB:
+ case DRM_FORMAT_ABGR:
+ case DRM_FORMAT_XRGB2101010:
+ case DRM_FORMAT_XBGR2101010:
+ case DRM_FORMAT_YUYV:
+ case DRM_FORMAT_YVYU:
+ case DRM_FORMAT_UYVY:
+ case DRM_FORMAT_VYUY:
+ /* All i915 modifiers are fine */
+ switch (modifier) {
+ case DRM_FORMAT_MOD_LINEAR:
+ case I915_FORMAT_MOD_X_TILED:
+ case I915_FORMAT_MOD_Y_TILED:
+ case I915_FORMAT_MOD_Yf_TILED:
+ return true;
+ default:
+ return false;
+ }
+ default:
+ return false;
+ }
+}
+
+static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane,
+uint32_t format,
+uint64_t modifier)
+{
+ struct drm_i915_private *dev_priv = to_i915(plane->dev);
+
+ if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
+ return false;
+
+ if (INTEL_GEN(dev_priv) >= 9)
+ return skl_mod_supported(format, modifier);
+ else if (INTEL_GEN(dev_priv) >= 4)
+ return i965_mod_supported(format, modifier);
+ else
+ return i8xx_mod_supported(format, modifier);
+
+ return false;
+}
+
+static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
+ uint32_t format,
+ uint64_t modifier)
+{
+ if (WARN_ON(modifier
[PATCH 2/3] drm: Create a format/modifier blob
Updated blob layout (Rob, Daniel, Kristian, xerpi)
Cc: Rob Clark
Cc: Daniel Stone
Cc: Kristian H. Kristensen
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/drm_mode_config.c | 7 +++
drivers/gpu/drm/drm_plane.c | 119 ++
include/drm/drm_mode_config.h | 6 ++
include/uapi/drm/drm_mode.h | 26 +
4 files changed, 158 insertions(+)
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index d9862259a2a7..6bfbc3839df5 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -337,6 +337,13 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.gamma_lut_size_property = prop;
+ prop = drm_property_create(dev,
+ DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
+ "IN_FORMATS", 0);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.modifiers = prop;
+
return 0;
}
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 286e183891e5..2e89e0e73435 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -62,6 +62,117 @@ static unsigned int drm_num_planes(struct drm_device *dev)
return num;
}
+struct drm_format_modifier_blob {
+#define FORMAT_BLOB_CURRENT 1
+ /* Version of this blob format */
+ u32 version;
+
+ /* Flags */
+ u32 flags;
+
+ /* Number of fourcc formats supported */
+ u32 count_formats;
+
+ /* Where in this blob the formats exist (in bytes) */
+ u32 formats_offset;
+
+ /* Number of drm_format_modifiers */
+ u32 count_modifiers;
+
+ /* Where in this blob the modifiers exist (in bytes) */
+ u32 modifiers_offset;
+
+ /* u32 formats[] */
+ /* struct drm_format_modifier modifiers[] */
+} __packed;
+
+static inline u32 *
+formats_ptr(struct drm_format_modifier_blob *blob)
+{
+ return (u32 *)(((char *)blob) + blob->formats_offset);
+}
+
+static inline struct drm_format_modifier *
+modifiers_ptr(struct drm_format_modifier_blob *blob)
+{
+ return (struct drm_format_modifier *)(((char *)blob) +
blob->modifiers_offset);
+}
+
+static int create_in_format_blob(struct drm_device *dev, struct drm_plane
*plane,
+const struct drm_plane_funcs *funcs,
+const uint32_t *formats, unsigned int
format_count,
+const uint64_t *format_modifiers)
+{
+ const struct drm_mode_config *config = &dev->mode_config;
+ const uint64_t *temp_modifiers = format_modifiers;
+ unsigned int format_modifier_count = 0;
+ struct drm_property_blob *blob = NULL;
+ struct drm_format_modifier *mod;
+ size_t blob_size = 0, formats_size, modifiers_size;
+ struct drm_format_modifier_blob *blob_data;
+ int i, j, ret = 0;
+
+ if (format_modifiers)
+ while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
+ format_modifier_count++;
+
+ formats_size = sizeof(__u32) * format_count;
+ if (WARN_ON(!formats_size)) {
+ /* 0 formats are never expected */
+ return 0;
+ }
+
+ modifiers_size =
+ sizeof(struct drm_format_modifier) * format_modifier_count;
+
+ blob_size = ALIGN(sizeof(struct drm_format_modifier_blob), 8);
+ blob_size += ALIGN(formats_size, 8);
+ blob_size += modifiers_size;
+
+ blob = drm_property_create_blob(dev, blob_size, NULL);
+ if (IS_ERR(blob))
+ return -1;
+
+ blob_data = (struct drm_format_modifier_blob *)blob->data;
+ blob_data->version = FORMAT_BLOB_CURRENT;
+ blob_data->count_formats = format_count;
+ blob_data->formats_offset = sizeof(struct drm_format_modifier_blob);
+ blob_data->count_modifiers = format_modifier_count;
+
+ /* Modifiers offset is a pointer to a struct with a 64 bit field so it
+* should be naturally aligned to 8B.
+*/
+ blob_data->modifiers_offset =
+ ALIGN(blob_data->formats_offset + formats_size, 8);
+
+ memcpy(formats_ptr(blob_data), formats, formats_size);
+
+ /* If we can't determine support, just bail */
+ if (!funcs->format_mod_supported)
+ goto done;
+
+ mod = modifiers_ptr(blob_data);
+ for (i = 0; i < format_modifier_count; i++) {
+ for (j = 0; j < format_count; j++) {
+ if (funcs->format_mod_supported(plane, formats[j],
+ format_modifiers[i])) {
+ mod->formats |= 1 << j;
+ }
+ }
+
+ mod->modifier = format_modifiers[i];
+ mod->offset = 0;
+ mod->pad = 0;
+
[PATCH 1/3] drm: Plumb modifiers through plane init
v2: A minor addition from Daniel
Cc: Daniel Stone
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/arc/arcpgu_crtc.c | 1 +
drivers/gpu/drm/arm/hdlcd_crtc.c| 1 +
drivers/gpu/drm/arm/malidp_planes.c | 2 +-
drivers/gpu/drm/armada/armada_crtc.c| 1 +
drivers/gpu/drm/armada/armada_overlay.c | 1 +
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 4 +++-
drivers/gpu/drm/drm_modeset_helper.c| 1 +
drivers/gpu/drm/drm_plane.c | 32 -
drivers/gpu/drm/drm_simple_kms_helper.c | 3 +++
drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +-
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 1 +
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
drivers/gpu/drm/i915/intel_display.c| 7 +-
drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
drivers/gpu/drm/imx/ipuv3-plane.c | 4 ++--
drivers/gpu/drm/mediatek/mtk_drm_plane.c| 2 +-
drivers/gpu/drm/meson/meson_plane.c | 1 +
drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 2 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++--
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +-
drivers/gpu/drm/nouveau/nv50_display.c | 5 ++--
drivers/gpu/drm/omapdrm/omap_plane.c| 3 ++-
drivers/gpu/drm/qxl/qxl_display.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 ++--
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++--
drivers/gpu/drm/sti/sti_cursor.c| 2 +-
drivers/gpu/drm/sti/sti_gdp.c | 2 +-
drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
drivers/gpu/drm/sun4i/sun4i_layer.c | 2 +-
drivers/gpu/drm/tegra/dc.c | 12 +-
drivers/gpu/drm/vc4/vc4_plane.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 4 ++--
drivers/gpu/drm/zte/zx_plane.c | 2 +-
include/drm/drm_plane.h | 21 +++-
include/drm/drm_simple_kms_helper.h | 1 +
include/uapi/drm/drm_fourcc.h | 11 +
41 files changed, 126 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c
b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ad9a95916f1f..cd8a24c7c67d 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -218,6 +218,7 @@ static struct drm_plane *arc_pgu_plane_init(struct
drm_device *drm)
ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs,
formats, ARRAY_SIZE(formats),
+ NULL,
DRM_PLANE_TYPE_PRIMARY, NULL);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 798a3cc480a2..0caa03ae8708 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -303,6 +303,7 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device
*drm)
ret = drm_universal_plane_init(drm, plane, 0xff, &hdlcd_plane_funcs,
formats, ARRAY_SIZE(formats),
+ NULL,
DRM_PLANE_TYPE_PRIMARY, NULL);
if (ret) {
devm_kfree(drm->dev, plane);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index 814fda23cead..b156610c68a5 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -400,7 +400,7 @@ int malidp_de_planes_init(struct drm_device *drm)
DRM_PLANE_TYPE_OVERLAY;
ret = drm_universal_plane_init(drm, &plane->base, crtcs,
&malidp_de_plane_funcs, formats,
- n, plane_type, NULL);
+ n, NULL, plane_type, NULL);
if (ret < 0)
goto cleanup;
diff --git a/drivers/gpu/drm/armada/armada_crtc.c
b/drivers/gpu/drm/armada/armada_crtc.c
index 4fe19fde84f9..ea48ef88f0e4 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -1269,6 +1269,7 @@ static int armada_drm_crtc_create(struct drm_device *drm,
struct device *dev,
&armada_primary_plane_funcs,
armada_primary_formats,
ARRAY_SIZE(armada_primary_
Re: [PATCH 1/3] drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN
On 03/05/17 12:06 AM, Gerd Hoffmann wrote: > >> Removing the definition also removes the possibility to describe a lot >> of pixel formats, so that should definitely be mentioned. I think it >> would also be good to have some kind of justified claim that no >> hardware actually needs the pixel formats this is removing (e.g. RGB565 >> BE). > > That and RGB2101010 BE are the only candidates I can think of. > > Dealing with those in none-native byteorder is a PITA though. Display > hardware is little endian (pci byte order) for quite a while already. Maybe by default, but not exclusively. > radeon looks differently on pre-R600 and R600+ hardware. > > pre-R600 can byteswap on cpu access, so the cpu view is bigendian > whereas things are actually stored on little endian byte order. > Hardware supports both 16bit and 32bit swaps. Used for fbdev emulation > (probably 32bit swaps, but not fully sure). 32-bit swaps for 32 bpp, 16-bit swaps for 16 bpp. > R600+ supports bigendian framebuffer formats, so no byteswapping on > access is needed. Not sure whenever that includes 16bpp formats or > whenever this is limited to the 8 bit-per-color formats [...] It includes 16bpp. Looking at drivers/gpu/drm/radeon/atombios_crtc.c:dce4_crtc_do_set_base(), it sets up byte-swapping for all multi-byte formats, so it effectively treats all those formats as if they had DRM_FORMAT_BIG_ENDIAN set. If the radeon (and amdgpu) driver were to be changed to use drm_mode_legacy_fb_format_he for >= R600, that must also handle 16 bpp, which requires DRM_FORMAT_BIG_ENDIAN. So I still don't see how that can be removed or even deprecated. From Ilia's followup it sounds like there's a similar situation with nouveau. -- 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 1/2] drm: Introduce crtc->mode_valid() callback
Hi Daniel,
On 02-05-2017 09:48, Daniel Vetter wrote:
> On Wed, Apr 26, 2017 at 11:48:34AM +0100, Jose Abreu wrote:
>> Some crtc's may have restrictions in the mode they can display. In
>> this patch a new callback (crtc->mode_valid()) is introduced that
>> is called at the same stage of connector->mode_valid() callback.
>>
>> This shall be implemented if the crtc has some sort of restriction
>> so that we don't probe modes that will fail in the commit() stage.
>> For example: A given crtc may be responsible to set a clock value.
>> If the clock can not produce all the values for the available
>> modes then this callback can be used to restrict the number of
>> probbed modes to only the ones that can be displayed.
>>
>> If the crtc does not implement the callback then the behaviour will
>> remain the same. Also, for a given set of crtcs that can be bound to
>> the connector, if at least one can display the mode then the mode
>> will be probbed.
>>
>> Signed-off-by: Jose Abreu
>> Cc: Carlos Palminha
>> Cc: Alexey Brodkin
>> Cc: Ville Syrjälä
>> Cc: Daniel Vetter
>> Cc: Dave Airlie
> Not sure this is useful, since you still have to duplicate the exact same
> check into your ->mode_fixup hook. That seems to make things even more
> confusing.
Yeah, in arcpgu I had to duplicate the code in ->atomic_check.
>
> Also this doesn't update the various kerneldoc comments. For the existing
> hooks. Since this topic causes so much confusion, I don't think a
> half-solution will help, but has some good potential to make things worse.
I only documented the callback in drm_modeset_helper_vtables.h.
Despite all of this, I think it doesn't makes sense delivering
modes to userspace which can never be used.
This is really annoying in arcpgu. Imagine: I try to use mpv to
play a video, the full set of modes from EDID were probed so if I
just start mpv it will pick the native mode of the TV instead of
the one that is supported, so mpv will fail to play. I know the
value of clock which will work (so I know what mode shall be
used), but a normal user which is not aware of the HW will have
to cycle through the list of modes and try them all until it hits
one that works. Its really boring.
For the modes that user specifies manually there is nothing we
can do, but we should not trick users into thinking that a given
mode is supported when it will always fail at commit.
Best regards,
Jose Miguel Abreu
> -Daniel
>
>> ---
>> drivers/gpu/drm/drm_probe_helper.c | 44
>>
>> include/drm/drm_modeset_helper_vtables.h | 26 +++
>> 2 files changed, 70 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c
>> b/drivers/gpu/drm/drm_probe_helper.c
>> index 1b0c14a..61eac30 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -80,6 +80,46 @@
>> return MODE_OK;
>> }
>>
>> +static enum drm_mode_status drm_mode_validate_connector_crtc(
>> +struct drm_connector *connector,
>> +struct drm_display_mode *mode)
>> +{
>> +const struct drm_crtc_helper_funcs *crtc_funcs = NULL;
>> +enum drm_mode_status mode_status = MODE_ERROR;
>> +struct drm_device *dev = connector->dev;
>> +struct drm_encoder *encoder;
>> +struct drm_crtc *crtc;
>> +bool callback_found = false;
>> +int i;
>> +
>> +for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
>> +encoder = drm_encoder_find(dev, connector->encoder_ids[i]);
>> +
>> +if (!encoder)
>> +continue;
>> +
>> +drm_for_each_crtc(crtc, dev) {
>> +crtc_funcs = crtc->helper_private;
>> +
>> +if (!drm_encoder_crtc_ok(encoder, crtc))
>> +continue;
>> +if (!crtc_funcs || !crtc_funcs->mode_valid)
>> +continue;
>> +
>> +/* MODE_OK=0 and default mode_status=MODE_ERROR=-1
>> + * so if at least one crtc accepts the mode we get
>> + * MODE_OK */
>> +mode_status &= crtc_funcs->mode_valid(crtc, mode);
>> +callback_found |= true;
>> +}
>> +}
>> +
>> +/* We can reach here without calling mode_valid if there is no
>> + * registered crtc with this callback, lets return MODE_OK in this
>> + * case */
>> +return callback_found ? mode_status : MODE_OK;
>> +}
>> +
>> static int drm_helper_probe_add_cmdline_mode(struct drm_connector
>> *connector)
>> {
>> struct drm_cmdline_mode *cmdline_mode;
>> @@ -431,6 +471,10 @@ int drm_helper_probe_single_connector_modes(struct
>> drm_connector *connector,
>> if (mode->status == MODE_OK && connector_funcs->mode_valid)
>> mode->status = connector_funcs->mode_valid(connector,
>> mode);
>> +
>> +if (mode->sta
Re: [PATCHv2 1/3] dt-bindings: display: Intel FPGA VIP drm driver Devicetree bindings
On Fri, 2017-04-28 at 13:32 -0500, Rob Herring wrote:
> On Tue, Apr 25, 2017 at 10:06:44AM +0800, hean.loong@intel.com
> wrote:
> >
> > From: "Ong, Hean Loong"
> >
> > Device tree binding for Intel FPGA Video and Image
> > Processing Suite. The binding involved would be generated
> > from the Altera (Intel) Qsys system. The bindings would
> > set the max width, max height, buts per pixel and memory
> > port width. The device tree binding only supports the Intel
> > Arria10 devkit and its variants. Vendor name retained as
> > altr.
> >
> > Signed-off-by: Ong, Hean Loong
> > ---
> > v2:
> > * Moved Device Tree bindings to
> > Documentation/devicetree/bindings/display/
> > * Added vendor name altr, to description
> > ---
> > .../devicetree/bindings/display/altr,vip-fb2.txt | 30
> > ++
> > 1 file changed, 30 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/altr,vip-
> > fb2.txt b/Documentation/devicetree/bindings/display/altr,vip-
> > fb2.txt
> > new file mode 100644
> > index 000..bdffefb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> > @@ -0,0 +1,30 @@
> > +Intel Video and Image Processing(VIP) Frame Buffer II bindings
> > +
> > +Supported hardware: Arria 10 and above with display port IP
> > +
> > +The drm driver for the Arria 10 devkit would require the display
> > resolution
> Bindings describe h/w. DRM driver is a Linux term.
>
Noted.
> >
> > +and pixel information to be included as these values are generated
> > based
> > +on the FPGA design that drives the video connector attached to the
> > drm driver
> > +Information the FPGA video IP component can be acquired from
> > +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/li
> > terature/ug/ug_vip.pdf
> > +
> > +Required properties:
> > +
> > +- compatible: "altr,vip-frame-buffer-2.0"
> > +- reg: Physical base address and length of the framebuffer
> > controller's
> > + registers.
> > +- altr,max-width: The width of the framebuffer in pixels.
> > +- altr,max-height: The height of the framebuffer in pixels.
> > +- altr,bits-per-symbol: only "8" is currently supported
> Supported in the driver or IP? The former isn't relevant to the
> binding.
> In the latter case, you don't need it if that's the only thing
> supported.
>
Since the device is an FPGA the values here are based on how the FPGA
HW design is created or programmed. The values here are the optimal
reference design proposed for the Intel Arria10 devkit.
However anyone that uses the Intel Arria10 devkit could create a device
that runs with a different resolution that has varying values but with
the condition that they need to fill these values accordingly with
Intel Quartus Programmer tools.
Once programmed the parameters could not be changed at runtime and the
HW rerence designs currently only support 1 type of resolution per
design.
Therefore the driver needs to support a hardware with varying
parameters programmed specifically into the FPGA.
> >
> > +- altr,mem-port-width = the bus width of the avalon master port on
> > the frame reader
> In bits or bytes?
>
> >
> > +
> > +Example:
> > +
> > + dp_0_frame_buf: vip@10280 {
> > + compatible = "altr,vip-frame-buffer-2.0";
> > + reg = <0x0001 0x0280 0x0040>;
> > + altr,max-width = <1280>;
> > + altr,max-height = <720>;
> > + altr,bits-per-symbol = <8>;
> > + altr,mem-port-width = <128>;
> > + };
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH v5 2/5] nouveau_hwmon: Add nouveau_hwmon_ops structure with .is_visible/.read_string
2017-05-02 6:25 GMT+02:00 Martin Peres :
> On 26/04/17 19:46, Oscar Salvador wrote:
>> This patch introduces the nouveau_hwmon_ops structure, sets up
>> .is_visible and .read_string operations and adds all the functions
>> for these operations.
>> This is also a preparation for the next patches, where most of the
>> work is being done.
>> This code doesn't interacture with the old one.
>> It's just to make easier the review of all patches.
>>
>> Signed-off-by: Oscar Salvador
>> ---
>> drivers/gpu/drm/nouveau/nouveau_hwmon.c | 170
>>
>> 1 file changed, 170 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> index 24b40c5..e8ea8d0 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> @@ -764,6 +764,176 @@ static const struct hwmon_channel_info *nouveau_info[]
>> = {
>> &nouveau_power,
>> NULL
>> };
>> +
>> +static umode_t
>> +nouveau_chip_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + switch (attr) {
>> + case hwmon_chip_update_interval:
>> + return 0444;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_power_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
>> +
>> + if (!iccsense)
>> + return 0;
>> +
>> + switch (attr) {
>> + case hwmon_power_input:
>> + if (iccsense->data_valid &&
>> + !list_empty(&iccsense->rails))
>
> Not sure if the kernel coding style would mandate !list_empty to be
> aligned with the iccsense in the above line, but this is our coding
> style at least.
>
> Anyway, this condition has to be moved before the switch as we should
> not expose power_max/crit if power_input is not exposed.
>
well, we could expose those anyway to let the user know what the GPU
is expected to consume at most. But other than that it is quite
useless, true.
>
>> + return 0444;
>> + return 0;
>> + case hwmon_power_max:
>> + if (iccsense->power_w_max)
>> + return 0444;
>> + return 0;
>> + case hwmon_power_crit:
>> + if (iccsense->power_w_crit)
>> + return 0444;
>> + return 0;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_temp_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>> +
>> + if (therm &&
>> + therm->attr_get &&
>> + nvkm_therm_temp_get(therm) < 0)
>> + return 0;
>
> You can also fold therm->attr_get on the same line as therm.
>
>> +
>> + switch (attr) {
>> + case hwmon_temp_input:
>> + case hwmon_temp_max:
>> + case hwmon_temp_max_hyst:
>> + case hwmon_temp_crit:
>> + case hwmon_temp_crit_hyst:
>> + case hwmon_temp_emergency:
>> + case hwmon_temp_emergency_hyst:
>> + return 0444;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_pwm_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>> +
>> + if (therm &&
>> + therm->attr_get &&
>> + therm->fan_get &&
>> + therm->fan_get(therm) < 0)
>> + return 0;
>
> Same, please fold as much as you can in 80 characters ;)
>
>> +
>> + switch (attr) {
>> + case hwmon_pwm_enable:
>> + case hwmon_pwm_input:
>> + return 0644;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_input_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
>> +
>> + if (!volt || nvkm_volt_get(volt) < 0)
>> + return 0;
>> +
>> + switch (attr) {
>> + case hwmon_in_input:
>> + case hwmon_in_label:
>> + case hwmon_in_min:
>> + case hwmon_in_max:
>> + return 0444;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_fan_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>> +
>> + if (!therm || !therm->attr_get || nvkm_therm_fan_sense(therm) < 0)
>> + return 0;
>> +
>> + switch (attr) {
>> + case hwmon_
linux-4.11/drivers/gpu/drm/bridge/tc358767.c:715]: (error) Signed integer overflow for expression 'max_tu_symbol<<23'.
Hello there, Source code is max_tu_symbol = TU_SIZE_RECOMMENDED - 1; tc_write(DP0_MISC, (max_tu_symbol << 23) | TU_SIZE_RECOMMENDED | BPC_8); and #define TU_SIZE_RECOMMENDED (0x3f << 16) /* LSCLK cycles per TU */ So it looks to me like 44 bits are required. Suggest use type long ? Regards David Binderman ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
drivers/gpu/drm/nouveau/nvkm/engine/dma/usernv04.c:124:: possible unintended fallthrough ?
Hello there,
drivers/gpu/drm/nouveau/nvkm/engine/dma/usernv04.c:124:18: warning: this
statement may fall through [-Wimplicit-fallthrough=]
Source code is
switch (dmaobj->base.access) {
case NV_MEM_ACCESS_RO:
dmaobj->flags0 |= 0x4000;
break;
case NV_MEM_ACCESS_WO:
dmaobj->flags0 |= 0x8000;
case NV_MEM_ACCESS_RW:
dmaobj->flags2 |= 0x0002;
break;
default:
return -EINVAL;
}
Suggest either document the fallthrough or add the missing break.
Regards
David Binderman
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH v5 5/5] nouveau_hwmon: Change permissions to numeric
2017-05-02 7:07 GMT+02:00 Martin Peres :
> On 26/04/17 19:46, Oscar Salvador wrote:
>> This patch replaces the symbolic permissions with the numeric ones,
>> and adds me to the authors too.
>>
>> Signed-off-by: Oscar Salvador
>
>
>> ---
>> drivers/gpu/drm/nouveau/nouveau_hwmon.c | 14 +++---
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> index 9142779..45b5c85 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
>> @@ -1,5 +1,6 @@
>> /*
>> - * Copyright 2010 Red Hat Inc.
>> + * Copyright 2010 Red Hat Inc. (Ben Skeggs)
>
> Please drop this change.
>
>> + * Copyright 2017 Oscar Salvador
>> *
>> * Permission is hereby granted, free of charge, to any person obtaining a
>> * copy of this software and associated documentation files (the
>> "Software"),
>> @@ -19,7 +20,6 @@
>> * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> * OTHER DEALINGS IN THE SOFTWARE.
>> *
>> - * Authors: Ben Skeggs
>
> You can't remove people as being the author of something. Just add
> yourself if you care about this (I did not care to add my name when I
> wrote in this file, because the git history makes more sense nowadays.
>
> Otherwise, I have no strong opinions on this patch. I guess the numeric
> representation is easier to read, so I will give you my R-b for this and
> let others decide what to do:
>
copyright holder != Author though. So yeah, he isn't allowed to change that.
> Reviewed-by: Martin Peres
>
>> */
>>
>> #ifdef CONFIG_ACPI
>> @@ -56,7 +56,7 @@ nouveau_hwmon_show_temp1_auto_point1_pwm(struct device *d,
>> {
>> return snprintf(buf, PAGE_SIZE, "%d\n", 100);
>> }
>> -static SENSOR_DEVICE_ATTR(temp1_auto_point1_pwm, S_IRUGO,
>> +static SENSOR_DEVICE_ATTR(temp1_auto_point1_pwm, 0444,
>> nouveau_hwmon_show_temp1_auto_point1_pwm, NULL, 0);
>>
>> static ssize_t
>> @@ -88,7 +88,7 @@ nouveau_hwmon_set_temp1_auto_point1_temp(struct device *d,
>>
>> return count;
>> }
>> -static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp, S_IRUGO | S_IWUSR,
>> +static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp, 0644,
>> nouveau_hwmon_temp1_auto_point1_temp,
>> nouveau_hwmon_set_temp1_auto_point1_temp, 0);
>>
>> @@ -121,7 +121,7 @@ nouveau_hwmon_set_temp1_auto_point1_temp_hyst(struct
>> device *d,
>>
>> return count;
>> }
>> -static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp_hyst, S_IRUGO | S_IWUSR,
>> +static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp_hyst, 0644,
>> nouveau_hwmon_temp1_auto_point1_temp_hyst,
>> nouveau_hwmon_set_temp1_auto_point1_temp_hyst, 0);
>>
>> @@ -255,7 +255,7 @@ nouveau_hwmon_set_pwm1_min(struct device *d, struct
>> device_attribute *a,
>> return count;
>> }
>>
>> -static SENSOR_DEVICE_ATTR(pwm1_min, S_IRUGO | S_IWUSR,
>> +static SENSOR_DEVICE_ATTR(pwm1_min, 0644,
>> nouveau_hwmon_get_pwm1_min,
>> nouveau_hwmon_set_pwm1_min, 0);
>>
>> @@ -295,7 +295,7 @@ nouveau_hwmon_set_pwm1_max(struct device *d, struct
>> device_attribute *a,
>> return count;
>> }
>>
>> -static SENSOR_DEVICE_ATTR(pwm1_max, S_IRUGO | S_IWUSR,
>> +static SENSOR_DEVICE_ATTR(pwm1_max, 0644,
>> nouveau_hwmon_get_pwm1_max,
>> nouveau_hwmon_set_pwm1_max, 0);
>>
>>
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 4/4] drm: zte: add VGA driver support
Hi Sean, On Tue, Apr 11, 2017 at 7:30 PM, Shawn Guo wrote: > From: Shawn Guo > > It adds VGA driver support, which needs to configure corresponding VOU > interface in RGB_888 format, and thus the following changes are needed > on zx_vou. > > - Rename the CSC block of Graphic Layer a bit to make it more specific, >and add CSC of Channel to support RGB output. > - Bypass Dither block for RGB output. > > Signed-off-by: Shawn Guo > Acked-by: Daniel Vetter > --- > Changes for v3: > - Set device state into disconnected when edid read fails in .get_modes >hook. > - Drop locking and add comments for understanding why it's not >necessary. Are you okay with this version? I need a 'go' from you to push the series to drm-misc-next. Shawn ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:amd-staging-4.9 3/17] drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:531:5: warning: this decimal constant is unsigned only in ISO C90
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head: 677efd69b448d24dd74e0dfb1d74fb6408c9df81
commit: 9ca8070c97b1eb2b948f065fdbccc15b6e8be639 [3/17] drm/amd/display: rename
bandwidth_calcs.c to dce_calcs.c (v2)
config: parisc-allyesconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 9ca8070c97b1eb2b948f065fdbccc15b6e8be639
# save the attached .config to linux build tree
make.cross ARCH=parisc
All warnings (new ones prefixed by >>):
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c: In function
'calculate_bandwidth':
>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:531:5: warning:
>> this decimal constant is unsigned only in ISO C90
data->lb_line_pitch =
bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875, 1),
bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))),
bw_int_to_fixed(48));
^~~~
vim +531 drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 515 }
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 516 }
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 517 }
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 518 }
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 519 }
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 520 lb_size_check = bw_def_ok;
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 521 for (i = 0; i <= maximum_number_of_surfaces - 1; i++) {
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 522 if (data->enable[i]) {
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 523 if ((dceip->pre_downscaler_enabled &&
bw_mtn(data->hsr[i], bw_int_to_fixed(1 {
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 524 data->source_width_in_lb =
bw_div(data->source_width_pixels[i], data->hsr[i]);
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 525 }
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 526 else {
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 527 data->source_width_in_lb =
data->source_width_pixels[i];
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 528 }
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 529 switch (data->lb_bpc[i]) {
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 530 case 8:
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 @531 data->lb_line_pitch =
bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875, 1),
bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))),
bw_int_to_fixed(48));
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 532 break;
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 533 case 10:
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 534 data->lb_line_pitch =
bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(300234375, 1000),
bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))),
bw_int_to_fixed(48));
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 535 break;
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 536 default:
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
2015-11-25 537 data->lb_line_pitch =
bw_ceil2(bw_mul(bw_int_to_fixed(data->lb_bpc[i]), data->source_width_in_lb),
bw_int_to_fixed(48));
bad4c165 drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c Harry Wentland
201
Re: [PATCH 2/4] PCI: add functionality for resizing resources v3
On Tue, May 2, 2017 at 6:51 PM, Christian König wrote:
> Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:
>> On Tue, Apr 25, 2017 at 4:19 PM, Christian König
>> wrote:
>>> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long
>>> type)
>>> +{
>>> + const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
>>> + IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
>>> +
>>
>> Redundant.
>
>
> Redundant, but also a reminder to myself that I wanted to ask something
> about that.
Missed context I suppose. Usually I comment in one word something
obvious, i.e. redundant empty line.
Sorry for missing my point.
> This type_mask is used already three times in this file, shouldn't we add a
> define for that?
Yes, that's wxactly what I commented somewhere (in one of the rest cases).
--
With Best Regards,
Andy Shevchenko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv3 2/3] drm/prime: Introduce drm_gem_prime_import_platform
On Tue, May 02, 2017 at 10:02:07AM -0700, Laura Abbott wrote:
> The existing drm_gem_prime_import function uses the underlying
> struct device of a drm_device for attaching to a dma_buf. Some drivers
> (notably vgem) may not have an underlying device structure. Offer
> an alternate function to attach using a platform device associated
> with drm_device.
>
> Cc: intel-...@lists.freedesktop.org
> Reviewed-by: Chris Wilson
> Signed-off-by: Laura Abbott
> ---
> v3: Rebase to drm-misc-next. Prototype moved to a new header file. Comments
> added for new function. Brought back drm_device->platformdev as it had been
> removed in 76adb460fd93 ("drm: Remove the struct drm_device platformdev
> field").
> I'm not entirely thrilled about this since the platformdev removal was good
> cleanup and this feels like a small step backwards. I don't know of a better
> approach though.
> ---
> drivers/gpu/drm/drm_prime.c | 49
> +++--
> include/drm/drmP.h | 2 ++
> include/drm/drm_prime.h | 4
> 3 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 9fb65b7..a557a4b 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -594,16 +594,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
>
> -/**
> - * drm_gem_prime_import - helper library implementation of the import
> callback
> - * @dev: drm_device to import into
> - * @dma_buf: dma-buf object to import
> - *
> - * This is the implementation of the gem_prime_import functions for GEM
> drivers
> - * using the PRIME helpers.
> - */
> -struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> - struct dma_buf *dma_buf)
> +static struct drm_gem_object *__drm_gem_prime_import(struct drm_device *dev,
> + struct dma_buf *dma_buf,
> + struct device *attach_dev)
> {
> struct dma_buf_attachment *attach;
> struct sg_table *sgt;
> @@ -625,7 +618,7 @@ struct drm_gem_object *drm_gem_prime_import(struct
> drm_device *dev,
> if (!dev->driver->gem_prime_import_sg_table)
> return ERR_PTR(-EINVAL);
>
> - attach = dma_buf_attach(dma_buf, dev->dev);
> + attach = dma_buf_attach(dma_buf, attach_dev);
> if (IS_ERR(attach))
> return ERR_CAST(attach);
>
> @@ -655,9 +648,43 @@ struct drm_gem_object *drm_gem_prime_import(struct
> drm_device *dev,
>
> return ERR_PTR(ret);
> }
> +
> +/**
> + * drm_gem_prime_import - helper library implementation of the import
> callback
> + * @dev: drm_device to import into
> + * @dma_buf: dma-buf object to import
> + *
> + * This is the implementation of the gem_prime_import functions for GEM
> drivers
> + * using the PRIME helpers.
> + */
> +struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + return __drm_gem_prime_import(dev, dma_buf, dev->dev);
> +}
> EXPORT_SYMBOL(drm_gem_prime_import);
>
> /**
> + * drm_gem_prime_import_platform - alternate implementation of the import
> callback
> + * @dev: drm_device to import into
> + * @dma_buf: dma-buf object to import
> + *
> + * This is identical to drm_gem_prime_import except the device used for
> dma_buf
> + * attachment is an internal platform device instead of the standard device
> + * structure. The use of this function should be limited to drivers that do
> not
> + * set up an underlying device structure.
> + */
> +struct drm_gem_object *drm_gem_prime_import_platform(struct drm_device *dev,
Simpler soluation will be for the caller to provide the platformdev?
That works nicely for the vgem case, I think.
> + struct dma_buf *dma_buf)
> +{
> + if (WARN_ON_ONCE(!dev->platformdev))
> + return NULL;
> +
> + return __drm_gem_prime_import(dev, dma_buf, &dev->platformdev->dev);
> +}
> +EXPORT_SYMBOL(drm_gem_prime_import_platform);
--
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] GPU-DRM-Radeon: Use seq_putc() in radeon_sa_bo_dump_debug_info()
From: Markus Elfring
Date: Tue, 2 May 2017 21:35:48 +0200
A few single characters should be put into a sequence.
Thus use the corresponding function "seq_putc".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/radeon/radeon_sa.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_sa.c
b/drivers/gpu/drm/radeon/radeon_sa.c
index 197b157b73d0..67bc3618798d 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -406,18 +406,15 @@ void radeon_sa_bo_dump_debug_info(struct
radeon_sa_manager *sa_manager,
list_for_each_entry(i, &sa_manager->olist, olist) {
uint64_t soffset = i->soffset + sa_manager->gpu_addr;
uint64_t eoffset = i->eoffset + sa_manager->gpu_addr;
- if (&i->olist == sa_manager->hole) {
- seq_printf(m, ">");
- } else {
- seq_printf(m, " ");
- }
+
+ seq_putc(m, (&i->olist == sa_manager->hole) ? '>' : ' ');
seq_printf(m, "[0x%010llx 0x%010llx] size %8lld",
soffset, eoffset, eoffset - soffset);
if (i->fence) {
seq_printf(m, " protected by 0x%016llx on ring %d",
i->fence->seq, i->fence->ring);
}
- seq_printf(m, "\n");
+ seq_putc(m, '\n');
}
spin_unlock(&sa_manager->wq.lock);
}
--
2.12.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] GPU-DRM-Radeon: Use seq_puts() in radeon_debugfs_pm_info()
From: Markus Elfring
Date: Tue, 2 May 2017 21:50:14 +0200
Two strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/radeon/radeon_pm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
b/drivers/gpu/drm/radeon/radeon_pm.c
index 326ad068c15a..f84ddcc426c1 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -1869,13 +1869,14 @@ static int radeon_debugfs_pm_info(struct seq_file *m,
void *data)
if ((rdev->flags & RADEON_IS_PX) &&
(ddev->switch_power_state != DRM_SWITCH_POWER_ON)) {
- seq_printf(m, "PX asic powered off\n");
+ seq_puts(m, "PX asic powered off\n");
} else if (rdev->pm.dpm_enabled) {
mutex_lock(&rdev->pm.mutex);
if (rdev->asic->dpm.debugfs_print_current_performance_level)
radeon_dpm_debugfs_print_current_performance_level(rdev, m);
else
- seq_printf(m, "Debugfs support not implemented for this
asic\n");
+ seq_puts(m,
+"Debugfs support not implemented for this
asic\n");
mutex_unlock(&rdev->pm.mutex);
} else {
seq_printf(m, "default engine clock: %u0 kHz\n",
rdev->pm.default_sclk);
--
2.12.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] GPU-DRM-Radeon: Use seq_puts() in r100_debugfs_cp_csq_fifo()
From: Markus Elfring
Date: Tue, 2 May 2017 21:54:49 +0200
Strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/radeon/r100.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index c31e660e35db..09b88738aa07 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2992,19 +2992,19 @@ static int r100_debugfs_cp_csq_fifo(struct seq_file *m,
void *data)
seq_printf(m, "Indirect2 wptr %u\n", ib2_wptr);
/* FIXME: 0, 128, 640 depends on fifo setup see cp_init_kms
* 128 = indirect1_start * 8 & 640 = indirect2_start * 8 */
- seq_printf(m, "Ring fifo:\n");
+ seq_puts(m, "Ring fifo:\n");
for (i = 0; i < 256; i++) {
WREG32(RADEON_CP_CSQ_ADDR, i << 2);
tmp = RREG32(RADEON_CP_CSQ_DATA);
seq_printf(m, "rfifo[%04d]=0x%08X\n", i, tmp);
}
- seq_printf(m, "Indirect1 fifo:\n");
+ seq_puts(m, "Indirect1 fifo:\n");
for (i = 256; i <= 512; i++) {
WREG32(RADEON_CP_CSQ_ADDR, i << 2);
tmp = RREG32(RADEON_CP_CSQ_DATA);
seq_printf(m, "ib1fifo[%04d]=0x%08X\n", i, tmp);
}
- seq_printf(m, "Indirect2 fifo:\n");
+ seq_puts(m, "Indirect2 fifo:\n");
for (i = 640; i < ib1_wptr; i++) {
WREG32(RADEON_CP_CSQ_ADDR, i << 2);
tmp = RREG32(RADEON_CP_CSQ_DATA);
--
2.12.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] GPU-DRM-Radeon: Fine-tuning for three function implementations
From: Markus Elfring Date: Tue, 2 May 2017 22:00:02 +0200 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Use seq_putc() in radeon_sa_bo_dump_debug_info() Use seq_puts() in radeon_debugfs_pm_info() Use seq_puts() in r100_debugfs_cp_csq_fifo() drivers/gpu/drm/radeon/r100.c | 6 +++--- drivers/gpu/drm/radeon/radeon_pm.c | 5 +++-- drivers/gpu/drm/radeon/radeon_sa.c | 9 +++-- 3 files changed, 9 insertions(+), 11 deletions(-) -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: analogix_dp: Remove unused check and variables
On Sat, Apr 29, 2017 at 08:39:07PM +0800, Jeffy Chen wrote:
> Remove unused check and variables after:
> drm/rockchip: Set line flag config register in vop_crtc_enable
>
> Signed-off-by: Jeffy Chen
>
> ---
Applied to -misc-next, thanks.
Sean
>
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 10 +-
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 9bfdbc6..1bccd82 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -104,17 +104,9 @@ static void analogix_dp_psr_work(struct work_struct
> *work)
> {
> struct rockchip_dp_device *dp =
> container_of(work, typeof(*dp), psr_work);
> - struct drm_crtc *crtc = dp->encoder.crtc;
> - int psr_state = dp->psr_state;
> - int vact_end;
> int ret;
> unsigned long flags;
>
> - if (!crtc)
> - return;
> -
> - vact_end = crtc->mode.vtotal - crtc->mode.vsync_start +
> crtc->mode.vdisplay;
> -
> ret = rockchip_drm_wait_vact_end(dp->encoder.crtc,
>PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
> if (ret) {
> @@ -123,7 +115,7 @@ static void analogix_dp_psr_work(struct work_struct *work)
> }
>
> spin_lock_irqsave(&dp->psr_lock, flags);
> - if (psr_state == EDP_VSC_PSR_STATE_ACTIVE)
> + if (dp->psr_state == EDP_VSC_PSR_STATE_ACTIVE)
> analogix_dp_enable_psr(dp->dev);
> else
> analogix_dp_disable_psr(dp->dev);
> --
> 2.1.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN
On Tue, May 2, 2017 at 11:06 AM, Gerd Hoffmann wrote: > Radeon and nvidia (nv40) cards where mentioned. I'll try to summarize > (feel free to correct me if I'm wrong). > > nvidia has support for 8 bit-per-color formats only on bigendian hosts. > Not sure whenever this is a driver or hardware limitation. Let me just summarize the NVIDIA situation. First off, pre-nv50 and nv50+ are entirely different and unrelated beasts. The (pre-nv50) hardware has (a few) "big endian mode" bits. Those bits are kind of unrelated to each other and control their own "domains". One of the domains is reading of the scanout fb. So as a result, the hardware can scan out XRGB, RGB565, and XRGB1555 stored in either little or big endian packings, irrespective of the "mode" that other parts of the hardware are in. However there's the delicate little question of the GPU *generating* the data. These older GPUs don't have quite the format flexibility offered by newer hw. So only XRGB is supported, packed in whatever "mode" the whole PGRAPH unit is in. (I say this because things seem to work when rendering using the XRGB format while scanning out with the BE flag set.) There are no APIs for controlling the endianness of each engine in nouveau, so it ends up being in "big endian" mode on BE hosts, so the GPU can only render to big-endian-packed framebuffers. None of this applies to nv50+ hw. (Although it might in broad strokes.) Currently the driver is exposing XRGB and ARGB formats as that's what drm_crtc_init does for it. However the ARGB format doesn't work (and shouldn't be exposed, the alpha is meaningless on a single-plane setup), and the XRGB format is assumed to be packed in cpu host endian (and the "BE" bit is set accordingly). Hope this helps! -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3 3/3] drm/vgem: Enable dmabuf import interfaces
Enable the GEM dma-buf import interfaces in addition to the export
interfaces. This lets vgem be used as a test source for other allocators
(e.g. Ion).
Cc: intel-...@lists.freedesktop.org
Reviewed-by: Chris Wilson
Signed-off-by: Laura Abbott
---
v3: Minor fixes suggested by Chris Wilson
---
drivers/gpu/drm/vgem/vgem_drv.c | 133 +++-
drivers/gpu/drm/vgem/vgem_drv.h | 2 +
2 files changed, 106 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 727eed2..c254c80 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -34,6 +34,9 @@
#include
#include
#include
+
+#include
+
#include "vgem_drv.h"
#define DRIVER_NAME"vgem"
@@ -46,6 +49,11 @@ static void vgem_gem_free_object(struct drm_gem_object *obj)
{
struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
+ drm_free_large(vgem_obj->pages);
+
+ if (obj->import_attach)
+ drm_prime_gem_destroy(obj, vgem_obj->table);
+
drm_gem_object_release(obj);
kfree(vgem_obj);
}
@@ -56,26 +64,49 @@ static int vgem_gem_fault(struct vm_fault *vmf)
struct drm_vgem_gem_object *obj = vma->vm_private_data;
/* We don't use vmf->pgoff since that has the fake offset */
unsigned long vaddr = vmf->address;
- struct page *page;
-
- page = shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping,
- (vaddr - vma->vm_start) >> PAGE_SHIFT);
- if (!IS_ERR(page)) {
- vmf->page = page;
- return 0;
- } else switch (PTR_ERR(page)) {
- case -ENOSPC:
- case -ENOMEM:
- return VM_FAULT_OOM;
- case -EBUSY:
- return VM_FAULT_RETRY;
- case -EFAULT:
- case -EINVAL:
- return VM_FAULT_SIGBUS;
- default:
- WARN_ON_ONCE(PTR_ERR(page));
- return VM_FAULT_SIGBUS;
+ int ret;
+ loff_t num_pages;
+ pgoff_t page_offset;
+ page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
+
+ num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
+
+ if (page_offset > num_pages)
+ return VM_FAULT_SIGBUS;
+
+ if (obj->pages) {
+ get_page(obj->pages[page_offset]);
+ vmf->page = obj->pages[page_offset];
+ ret = 0;
+ } else {
+ struct page *page;
+
+ page = shmem_read_mapping_page(
+ file_inode(obj->base.filp)->i_mapping,
+ page_offset);
+ if (!IS_ERR(page)) {
+ vmf->page = page;
+ ret = 0;
+ } else switch (PTR_ERR(page)) {
+ case -ENOSPC:
+ case -ENOMEM:
+ ret = VM_FAULT_OOM;
+ break;
+ case -EBUSY:
+ ret = VM_FAULT_RETRY;
+ break;
+ case -EFAULT:
+ case -EINVAL:
+ ret = VM_FAULT_SIGBUS;
+ break;
+ default:
+ WARN_ON(PTR_ERR(page));
+ ret = VM_FAULT_SIGBUS;
+ break;
+ }
+
}
+ return ret;
}
static const struct vm_operations_struct vgem_gem_vm_ops = {
@@ -112,12 +143,8 @@ static void vgem_postclose(struct drm_device *dev, struct
drm_file *file)
kfree(vfile);
}
-/* ioctls */
-
-static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
- struct drm_file *file,
- unsigned int *handle,
- unsigned long size)
+static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device *dev,
+ unsigned long size)
{
struct drm_vgem_gem_object *obj;
int ret;
@@ -127,8 +154,31 @@ static struct drm_gem_object *vgem_gem_create(struct
drm_device *dev,
return ERR_PTR(-ENOMEM);
ret = drm_gem_object_init(dev, &obj->base, roundup(size, PAGE_SIZE));
- if (ret)
- goto err_free;
+ if (ret) {
+ kfree(obj);
+ return ERR_PTR(ret);
+ }
+
+ return obj;
+}
+
+static void __vgem_gem_destroy(struct drm_vgem_gem_object *obj)
+{
+ drm_gem_object_release(&obj->base);
+ kfree(obj);
+}
+
+static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
+ struct drm_file *file,
+
[PATCHv3 1/3] drm/vgem: Add a dummy platform device
The vgem driver is currently registered independent of any actual
device. Some usage of the dmabuf APIs require an actual device structure
to do anything. Register a dummy platform device for use with dmabuf.
Cc: intel-...@lists.freedesktop.org
Reviewed-by: Chris Wilson
Signed-off-by: Laura Abbott
---
v3: No changes
---
drivers/gpu/drm/vgem/vgem_drv.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 9fee38a..727eed2 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -335,11 +335,20 @@ static int __init vgem_init(void)
int ret;
vgem_device = drm_dev_alloc(&vgem_driver, NULL);
- if (IS_ERR(vgem_device)) {
- ret = PTR_ERR(vgem_device);
+ if (IS_ERR(vgem_device))
+ return PTR_ERR(vgem_device);
+
+ vgem_device->platformdev = platform_device_register_simple("vgem",
+ -1, NULL, 0);
+
+ if (!vgem_device->platformdev) {
+ ret = -ENODEV;
goto out;
}
+ dma_coerce_mask_and_coherent(&vgem_device->platformdev->dev,
+ DMA_BIT_MASK(64));
+
ret = drm_dev_register(vgem_device, 0);
if (ret)
goto out_unref;
@@ -347,13 +356,15 @@ static int __init vgem_init(void)
return 0;
out_unref:
- drm_dev_unref(vgem_device);
+ platform_device_unregister(vgem_device->platformdev);
out:
+ drm_dev_unref(vgem_device);
return ret;
}
static void __exit vgem_exit(void)
{
+ platform_device_unregister(vgem_device->platformdev);
drm_dev_unregister(vgem_device);
drm_dev_unref(vgem_device);
}
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3 0/3] dma_buf import support for vgem
Hi, This is v3 of the series to add dma_buf import functions for vgem. This is mostly a rebase to drm-misc/drm-misc-next with a fixup of the resulting conflicts. More details can be found on the individual patches. Thanks, Laura Laura Abbott (3): drm/vgem: Add a dummy platform device drm/prime: Introduce drm_gem_prime_import_platform drm/vgem: Enable dmabuf import interfaces drivers/gpu/drm/drm_prime.c | 49 ++--- drivers/gpu/drm/vgem/vgem_drv.c | 150 +++- drivers/gpu/drm/vgem/vgem_drv.h | 2 + include/drm/drmP.h | 2 + include/drm/drm_prime.h | 4 ++ 5 files changed, 164 insertions(+), 43 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3 2/3] drm/prime: Introduce drm_gem_prime_import_platform
The existing drm_gem_prime_import function uses the underlying
struct device of a drm_device for attaching to a dma_buf. Some drivers
(notably vgem) may not have an underlying device structure. Offer
an alternate function to attach using a platform device associated
with drm_device.
Cc: intel-...@lists.freedesktop.org
Reviewed-by: Chris Wilson
Signed-off-by: Laura Abbott
---
v3: Rebase to drm-misc-next. Prototype moved to a new header file. Comments
added for new function. Brought back drm_device->platformdev as it had been
removed in 76adb460fd93 ("drm: Remove the struct drm_device platformdev field").
I'm not entirely thrilled about this since the platformdev removal was good
cleanup and this feels like a small step backwards. I don't know of a better
approach though.
---
drivers/gpu/drm/drm_prime.c | 49 +++--
include/drm/drmP.h | 2 ++
include/drm/drm_prime.h | 4
3 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 9fb65b7..a557a4b 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -594,16 +594,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
-/**
- * drm_gem_prime_import - helper library implementation of the import callback
- * @dev: drm_device to import into
- * @dma_buf: dma-buf object to import
- *
- * This is the implementation of the gem_prime_import functions for GEM drivers
- * using the PRIME helpers.
- */
-struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
- struct dma_buf *dma_buf)
+static struct drm_gem_object *__drm_gem_prime_import(struct drm_device *dev,
+ struct dma_buf *dma_buf,
+ struct device *attach_dev)
{
struct dma_buf_attachment *attach;
struct sg_table *sgt;
@@ -625,7 +618,7 @@ struct drm_gem_object *drm_gem_prime_import(struct
drm_device *dev,
if (!dev->driver->gem_prime_import_sg_table)
return ERR_PTR(-EINVAL);
- attach = dma_buf_attach(dma_buf, dev->dev);
+ attach = dma_buf_attach(dma_buf, attach_dev);
if (IS_ERR(attach))
return ERR_CAST(attach);
@@ -655,9 +648,43 @@ struct drm_gem_object *drm_gem_prime_import(struct
drm_device *dev,
return ERR_PTR(ret);
}
+
+/**
+ * drm_gem_prime_import - helper library implementation of the import callback
+ * @dev: drm_device to import into
+ * @dma_buf: dma-buf object to import
+ *
+ * This is the implementation of the gem_prime_import functions for GEM drivers
+ * using the PRIME helpers.
+ */
+struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+ struct dma_buf *dma_buf)
+{
+ return __drm_gem_prime_import(dev, dma_buf, dev->dev);
+}
EXPORT_SYMBOL(drm_gem_prime_import);
/**
+ * drm_gem_prime_import_platform - alternate implementation of the import
callback
+ * @dev: drm_device to import into
+ * @dma_buf: dma-buf object to import
+ *
+ * This is identical to drm_gem_prime_import except the device used for dma_buf
+ * attachment is an internal platform device instead of the standard device
+ * structure. The use of this function should be limited to drivers that do not
+ * set up an underlying device structure.
+ */
+struct drm_gem_object *drm_gem_prime_import_platform(struct drm_device *dev,
+ struct dma_buf *dma_buf)
+{
+ if (WARN_ON_ONCE(!dev->platformdev))
+ return NULL;
+
+ return __drm_gem_prime_import(dev, dma_buf, &dev->platformdev->dev);
+}
+EXPORT_SYMBOL(drm_gem_prime_import_platform);
+
+/**
* drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
* @dev: dev to export the buffer from
* @file_priv: drm file-private structure
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index e1daa4f..086daee 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -439,6 +439,8 @@ struct drm_device {
struct pci_controller *hose;
#endif
+ /**< Platform device for drivers that do not use the standard device */
+ struct platform_device *platformdev;
struct virtio_device *virtdev;
struct drm_sg_mem *sg; /**< Scatter gather memory */
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 0b2a235..9fe39f8 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -65,6 +65,10 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
int *prime_fd);
struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
+
+struct drm_gem_object *drm_gem_prime_import_platform(struct drm_device *dev,
+struct dma_buf *dma
Re: [PATCH 2/4] PCI: add functionality for resizing resources v3
Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:
On Tue, Apr 25, 2017 at 4:19 PM, Christian König
wrote:
From: Christian König
This allows device drivers to request resizing their BARs.
The function only tries to reprogram the windows of the bridge directly above
the requesting device and only the BAR of the same type (usually mem, 64bit,
prefetchable). This is done to make sure not to disturb other drivers by
changing the BARs of their devices.
If reprogramming the bridge BAR fails the old status is restored and -ENOSPC
returned to the calling device driver.
+int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
+{
+ const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
+ IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
+
Redundant.
Redundant, but also a reminder to myself that I wanted to ask something
about that.
This type_mask is used already three times in this file, shouldn't we
add a define for that?
[SNIP]
+ list_for_each_entry(dev_res, &saved, list) {
+ /* Skip the bridge we just assigned resources for. */
+ if (bridge == dev_res->dev)
+ continue;
+
+ bridge = dev_res->dev;
+ pci_setup_bridge(bridge->subordinate);
+ }
+
+ free_list(&saved);
+ free_list(&failed);
+ return ret;
You might re-use two lines with below, but perhaps better to show
which case returns 0 explicitly and drop assignment ret = 0 above.
Good point, but actually the free_list(&failed) is superfluous here
since when the failed list isn't empty we end up in the cleanup path.
Going to fix all other comments in the next version.
Regards,
Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: ipu-v3: pre: only use internal clock gating
By setting the SFTRST bit, the PRE will be held in the lowest power state
with clocks to the internal blocks gated. When external clock gating is
used (from the external clock controller, or by setting the CLKGATE bit)
the PRE will sporadically fail to start.
Signed-off-by: Lucas Stach
---
This is a fix for newly introduced functionality and should be applied
for 4.12 fixes.
---
drivers/gpu/ipu-v3/ipu-pre.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
index c55563379e2e..e2f6acf42792 100644
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -131,8 +131,6 @@ int ipu_pre_get(struct ipu_pre *pre)
if (pre->in_use)
return -EBUSY;
- clk_prepare_enable(pre->clk_axi);
-
/* first get the engine out of reset and remove clock gating */
writel(0, pre->regs + IPU_PRE_CTRL);
@@ -149,12 +147,7 @@ int ipu_pre_get(struct ipu_pre *pre)
void ipu_pre_put(struct ipu_pre *pre)
{
- u32 val;
-
- val = IPU_PRE_CTRL_SFTRST | IPU_PRE_CTRL_CLKGATE;
- writel(val, pre->regs + IPU_PRE_CTRL);
-
- clk_disable_unprepare(pre->clk_axi);
+ writel(IPU_PRE_CTRL_SFTRST, pre->regs + IPU_PRE_CTRL);
pre->in_use = false;
}
@@ -249,6 +242,8 @@ static int ipu_pre_probe(struct platform_device *pdev)
if (!pre->buffer_virt)
return -ENOMEM;
+ clk_prepare_enable(pre->clk_axi);
+
pre->dev = dev;
platform_set_drvdata(pdev, pre);
mutex_lock(&ipu_pre_list_mutex);
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN
Hi, > I also think that this patch requires more comments than the > commit message has at the moment. > > Removing the definition also removes the possibility to describe a lot > of pixel formats, so that should definitely be mentioned. I think it > would also be good to have some kind of justified claim that no > hardware actually needs the pixel formats this is removing (e.g. RGB565 > BE). That and RGB2101010 BE are the only candidates I can think of. Dealing with those in none-native byteorder is a PITA though. Display hardware is little endian (pci byte order) for quite a while already. > Maybe this was already in the long discussions, but I feel it > should be summarized in the commit message. Radeon and nvidia (nv40) cards where mentioned. I'll try to summarize (feel free to correct me if I'm wrong). nvidia has support for 8 bit-per-color formats only on bigendian hosts. Not sure whenever this is a driver or hardware limitation. radeon looks differently on pre-R600 and R600+ hardware. pre-R600 can byteswap on cpu access, so the cpu view is bigendian whereas things are actually stored on little endian byte order. Hardware supports both 16bit and 32bit swaps. Used for fbdev emulation (probably 32bit swaps, but not fully sure). xorg radeon driver doesn't use the byteswapping feature, because it is a PITA when bo's are moved between vram and system memory. R600+ supports bigendian framebuffer formats, so no byteswapping on access is needed. Not sure whenever that includes 16bpp formats or whenever this is limited to the 8 bit-per-color formats (simliar to nvidia). Discussion focused on the pre-R600 hardware and how this on-acpu-access byteswapping is more a problem than it helps ... cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] PCI: add resizeable BAR infrastructure v4
Am 26.04.2017 um 18:45 schrieb Andy Shevchenko: [SNIP] -#define PCI_REBAR_CTRL_NBAR_MASK (7 << 5)/* mask for # bars */ -#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # bars */ +#define PCI_REBAR_CTRL_NBAR_MASK (7 << 5)/* mask for # BARs */ +#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # BARs */ I understand why, but I dunno it worth to do. Bjorn suggested that. Either way is fine with me, but he needs to stick his signed-of-by on it when pushing it upstream. So his opinion usually wins. All other comments will be integrated in the next version of the patch. Thanks for the review, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/9] dt-bindings: display: sun4i: Add component endpoint ID numbering scheme
Hi, On Fri, Apr 28, 2017 at 08:48:41AM -0500, Rob Herring wrote: > On Fri, Apr 21, 2017 at 04:38:49PM +0800, Chen-Yu Tsai wrote: > > The Allwinner display pipeline contains many hardware components, some > > of which can consume data from one of multiple upstream components. > > The numbering scheme of these components must be encoded into the device > > tree so the driver can figure out which component out of two or more of > > the same type it is supposed to use or program. > > > > This patch adds the constraint that local endpoint IDs must be the index > > or number of the remote endpoint's hardware block, for all components > > in the display pipeline up to the TCONs. > > > > Signed-off-by: Chen-Yu Tsai > > --- > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 10 > > ++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > index 57a8d0610062..7acdbf14ae1c 100644 > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > @@ -4,6 +4,16 @@ Allwinner A10 Display Pipeline > > The Allwinner A10 Display pipeline is composed of several components > > that are going to be documented below: > > > > +For the input port of all components up to the TCON in the display > > +pipeline, if there are multiple components, the local endpoint IDs > > +must correspond to the index of the upstream block. For example, if > > +the remote endpoint is Frontend 1, then the local endpoint ID must > > +be 1. > > + > > +Conversely, for the output ports of the same group, the remote endpoint > > +ID must be the index of the local hardware block. If the local backend > > +is backend 1, then the remote endpoint ID must be 1. > > It would be clearer if you just explicitly listed IDs and their > connections. From how this is worded, it would not work if you had > connections like this: > > DevA 0 > DevA 1 > DevB 0 > DevB 1 > > These would need to be endpoints 0-3 in TCON, and that doesn't reflect > the remote devices' index. Chen-Yu, can you send a patch to rephrase the doc that way? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Proposal for RandR version 1.6, Leases and EDID-based output grabs
Pekka Paalanen writes: > I was under the impression that if you have a VR application running on > the HMD, it necessarily also implies that you have a VR compositor > running, which means that there is always some process providing a valid > image to the HMD. (At least in end-user setups.) Yes, I think the point I was trying to make was that X should never attempt to talk to the HMD and provide an image. > I assumed that means there are no VR apps nor a VR compositor that > could handle the HMD. In that case I think the HMD should be always > off. Agreed. > I presume that if "desktop" is set to "true", it implies that the HMD > is capable of showing a simple 2D canvas in stereo without any special > rendering and with the default video mode. That is, creating a sort of > a virtual 2D monitor. That would be nice. I was thinking that 'desktop' would be true for non-HMD displays that didn't need the VR compositor. If you've got a VR compositor and want to paint the desktop inside the VR environment, then I think you'd want to create a synthetic monitor and hand images from that to the VR compositor each frame. > FWIW, it all sounds good to me! > > I don't really have an opinion on XML vs. JSON, I'm just happy it's not > another ad hoc format. Yeah, I think we're done with ad-hoc formats. I've done both XML and JSON, and JSON is way easier to hand write, but we already use XML in a bunch of places, so the necessary libaries are already linked into the server... -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN
On Tue, 2 May 2017 14:53:43 +0100
Emil Velikov wrote:
> Hi Gerd,
>
> I did not have the change to follow through the discussion.
> Pardon if my suggestion have already been discussed.
>
> On 2 May 2017 at 14:34, Gerd Hoffmann wrote:
> > It's unused.
> >
> > Suggested-by: Daniel Vetter
> > Signed-off-by: Gerd Hoffmann
> > ---
> > include/uapi/drm/drm_fourcc.h | 2 --
> > drivers/gpu/drm/drm_fourcc.c | 3 +--
> > drivers/gpu/drm/drm_framebuffer.c | 2 +-
> > 3 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 995c8f9c69..305bc34be0 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -33,8 +33,6 @@ extern "C" {
> > #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
> > ((__u32)(c) << 16) | ((__u32)(d) << 24))
> >
> > -#define DRM_FORMAT_BIG_ENDIAN (1<<31) /* format is big endian instead of
> > little endian */
> > -
>
> Weston references DRM_FORMAT_BIG_ENDIAN thus this patch will lead to
> build breakage.
> That is never fun, so please carefully coordinate with the Weston devs
> to minimise the fireworks.
>
> Personally I would leave the symbol, since it's UAPI and document that
> should not be used. Not my call, so feel free to ignore.
Hi,
indeed, weston does have one occurrence of it. I don't think it has
actually been needed in practice ever, but removing it will cause a
build failure:
https://cgit.freedesktop.org/wayland/weston/tree/libweston/gl-renderer.c?id=2.0.0#n1820
Funnily enough, it's only ever used to get rid of the bit, "just in
case".
I also think that this patch requires more comments than the
commit message has at the moment.
Removing the definition also removes the possibility to describe a lot
of pixel formats, so that should definitely be mentioned. I think it
would also be good to have some kind of justified claim that no
hardware actually needs the pixel formats this is removing (e.g. RGB565
BE). Maybe this was already in the long discussions, but I feel it
should be summarized in the commit message.
Thanks,
pq
pgptinkXCgcMh.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 995c8f9c69..305bc34be0 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -33,8 +33,6 @@ extern "C" {
> > #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
> > ((__u32)(c) << 16) | ((__u32)(d) << 24))
> >
> > -#define DRM_FORMAT_BIG_ENDIAN (1<<31) /* format is big endian instead of
> > little endian */
> > -
>
> Weston references DRM_FORMAT_BIG_ENDIAN thus this patch will lead to
> build breakage.
> Personally I would leave the symbol, since it's UAPI and document that
> should not be used.
Fair enough. I can surely add a deprecated comment instead of just
dropping it.
I'm wondering how it is used though, given that none of the drivers in
the kernel actually support this flag ...
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN
Hi Gerd,
I did not have the change to follow through the discussion.
Pardon if my suggestion have already been discussed.
On 2 May 2017 at 14:34, Gerd Hoffmann wrote:
> It's unused.
>
> Suggested-by: Daniel Vetter
> Signed-off-by: Gerd Hoffmann
> ---
> include/uapi/drm/drm_fourcc.h | 2 --
> drivers/gpu/drm/drm_fourcc.c | 3 +--
> drivers/gpu/drm/drm_framebuffer.c | 2 +-
> 3 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 995c8f9c69..305bc34be0 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -33,8 +33,6 @@ extern "C" {
> #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
> ((__u32)(c) << 16) | ((__u32)(d) << 24))
>
> -#define DRM_FORMAT_BIG_ENDIAN (1<<31) /* format is big endian instead of
> little endian */
> -
Weston references DRM_FORMAT_BIG_ENDIAN thus this patch will lead to
build breakage.
That is never fun, so please carefully coordinate with the Weston devs
to minimise the fireworks.
Personally I would leave the symbol, since it's UAPI and document that
should not be used. Not my call, so feel free to ignore.
Thanks
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm: fourcc byteorder: add drm_mode_legacy_fb_format_he
Add drm_mode_legacy_fb_format variant which returns fourcc codes
for framebuffer format in host byte order.
Signed-off-by: Gerd Hoffmann
---
include/drm/drm_fourcc.h | 3 +++
drivers/gpu/drm/drm_fourcc.c | 54 +++-
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index cae05153e8..729e9d1b11 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -73,7 +73,10 @@ const struct drm_format_info *drm_format_info(u32 format);
const struct drm_format_info *
drm_get_format_info(struct drm_device *dev,
const struct drm_mode_fb_cmd2 *mode_cmd);
+
uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
+uint32_t drm_mode_legacy_fb_format_he(uint32_t bpp, uint32_t depth);
+
int drm_format_num_planes(uint32_t format);
int drm_format_plane_cpp(uint32_t format, int plane);
int drm_format_horz_chroma_subsampling(uint32_t format);
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index adb3ff59a4..97ff2cdf4e 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -36,12 +36,24 @@ static char printable_char(int c)
}
/**
- * drm_mode_legacy_fb_format - compute drm fourcc code from legacy description
+ * drm_mode_legacy_fb_format - compute drm fourcc code from legacy description,
+ * little endian.
* @bpp: bits per pixels
* @depth: bit depth per pixel
*
+ * Deprecated, use drm_mode_legacy_fb_format_he instead.
+ *
* Computes a drm fourcc pixel format code for the given @bpp/@depth values.
* Useful in fbdev emulation code, since that deals in those values.
+ *
+ * Note that drm_mode_addfb (DRM_IOCTL_MODE_ADDFB implementation) uses this
+ * too.
+ *
+ * For historical reasons, this function returns fourcc codes for framebuffer
+ * formats in little endian byte order unconditinally, even though fbdev
+ * emulation expects the framebuffer in host byte order (i.e. big endian on
+ * big endian machines). Ideally we would simply fix this function, but that
+ * would break drivers expecting the broken behavior ...
*/
uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
{
@@ -79,6 +91,46 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t
depth)
EXPORT_SYMBOL(drm_mode_legacy_fb_format);
/**
+ * drm_mode_legacy_fb_format_he - compute drm fourcc code from legacy
+ *description, host endian.
+ * @bpp: bits per pixels
+ * @depth: bit depth per pixel
+ *
+ * Computes a drm fourcc pixel format code for the given @bpp/@depth values.
+ * Useful in fbdev emulation code, since that deals in those values.
+ */
+uint32_t drm_mode_legacy_fb_format_he(uint32_t bpp, uint32_t depth)
+{
+#ifdef __BIG_ENDIAN
+ uint32_t fmt;
+
+ switch (bpp) {
+ case 8:
+ fmt = DRM_FORMAT_C8;
+ break;
+ case 24:
+ fmt = DRM_FORMAT_BGR888;
+ break;
+ case 32:
+ if (depth == 24)
+ fmt = DRM_FORMAT_BGRX;
+ else
+ fmt = DRM_FORMAT_BGRA;
+ break;
+ default:
+ DRM_ERROR("bad bpp, assuming b8g8r8x8 pixel format\n");
+ fmt = DRM_FORMAT_BGRX;
+ break;
+ }
+
+ return fmt;
+#else
+ return drm_mode_legacy_fb_format(bpp, depth);
+#endif
+}
+EXPORT_SYMBOL(drm_mode_legacy_fb_format_he);
+
+/**
* drm_get_format_name - fill a string with a drm fourcc format's name
* @format: format to compute name of
* @buf: caller-supplied buffer
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm: fourcc byteorder: add DRM_FORMAT_CPU_*
Add fourcc variants in cpu byte order. With these at hand we don't need #ifdefs in drivers want support framebuffers in cpu endianess. Signed-off-by: Gerd Hoffmann --- include/drm/drm_fourcc.h | 12 1 file changed, 12 insertions(+) diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index 6942e84b6e..cae05153e8 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -25,6 +25,18 @@ #include #include +/* + * DRM formats are little endian. define cpu endian variants here, to + * reduce the #ifdefs needed in drivers. + */ +#ifdef __BIG_ENDIAN +# define DRM_FORMAT_CPU_XRGB DRM_FORMAT_BGRX +# define DRM_FORMAT_CPU_ARGB DRM_FORMAT_BGRA +#else +# define DRM_FORMAT_CPU_XRGB DRM_FORMAT_XRGB +# define DRM_FORMAT_CPU_ARGB DRM_FORMAT_ARGB +#endif + struct drm_device; struct drm_mode_fb_cmd2; -- 2.9.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN
It's unused.
Suggested-by: Daniel Vetter
Signed-off-by: Gerd Hoffmann
---
include/uapi/drm/drm_fourcc.h | 2 --
drivers/gpu/drm/drm_fourcc.c | 3 +--
drivers/gpu/drm/drm_framebuffer.c | 2 +-
3 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 995c8f9c69..305bc34be0 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -33,8 +33,6 @@ extern "C" {
#define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
((__u32)(c) << 16) | ((__u32)(d) << 24))
-#define DRM_FORMAT_BIG_ENDIAN (1<<31) /* format is big endian instead of
little endian */
-
/* color index */
#define DRM_FORMAT_C8 fourcc_code('C', '8', ' ', ' ') /* [7:0] C */
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 9c0152df45..adb3ff59a4 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -86,12 +86,11 @@ EXPORT_SYMBOL(drm_mode_legacy_fb_format);
const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf
*buf)
{
snprintf(buf->str, sizeof(buf->str),
-"%c%c%c%c %s-endian (0x%08x)",
+"%c%c%c%c (0x%08x)",
printable_char(format & 0xff),
printable_char((format >> 8) & 0xff),
printable_char((format >> 16) & 0xff),
printable_char((format >> 24) & 0x7f),
-format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
format);
return buf->str;
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index fc8ef42203..efe8b5ece5 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -152,7 +152,7 @@ static int framebuffer_check(struct drm_device *dev,
int i;
/* check if the format is supported at all */
- info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
+ info = __drm_format_info(r->pixel_format);
if (!info) {
struct drm_format_name_buf format_name;
DRM_DEBUG_KMS("bad framebuffer format %s\n",
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] drm: tackle byteorder issues, take three ...
Hi, Ok, next round. Patches 1+2 are unmodified, driver updates are left out for now. Patch #3 adds a new drm_mode_legacy_fb_format_he() function instead of changing drm_mode_legacy_fb_format behavior, so we keep things working for now. Comments? Suggestions how to handle the drm_mode_legacy_fb_format() call in drm_mode_addfb() ? Add some driver flag? Gerd Hoffmann (3): drm: fourcc byteorder: drop DRM_FORMAT_BIG_ENDIAN drm: fourcc byteorder: add DRM_FORMAT_CPU_* drm: fourcc byteorder: add drm_mode_legacy_fb_format_he include/drm/drm_fourcc.h | 15 +++ include/uapi/drm/drm_fourcc.h | 2 -- drivers/gpu/drm/drm_fourcc.c | 57 --- drivers/gpu/drm/drm_framebuffer.c | 2 +- 4 files changed, 70 insertions(+), 6 deletions(-) -- 2.9.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100802] [regression] mostly blank graphics on Faeria
https://bugs.freedesktop.org/show_bug.cgi?id=100802 --- Comment #3 from Timo Aaltonen --- Ran more tests and tried to bisect mesa, but the end result is something weird that I can't figure out anymore where the bug is. Using whatever combination of kernel, llvm, mesa, or the radeon X driver on Ubuntu 16.04 seems to work, but fails on 17.04. Took an apitrace of running Faeria: https://aaltoset.kapsi.fi/tmp/Faeria.x86_64.trace.xz -- 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 1/2] drm: Add new DRM_IOCTL_MODE_GETPLANE2
Am Freitag, den 28.04.2017, 11:33 -0700 schrieb Ben Widawsky: > On 17-04-28 14:11:56, Lucas Stach wrote: > >Hi Ben, > > > >Am Dienstag, den 04.04.2017, 12:41 -0700 schrieb Ben Widawsky: > >> On 17-04-04 11:07:26, Daniel Stone wrote: > >> >Hi, > >> > > >> >On 1 April 2017 at 19:47, Rob Clark wrote: > >> >> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen > >> >> wrote: > >> >>> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning > >> >>> information about the modifiers that will work with each format. > >> >> > >> >> So, just to toss out a completely different idea.. > >> >> > >> >> What if instead of a new ioctl, we had a read-only blob property > >> >> (which encoded the info basically the same way, plus the fourcc's)? > >> >> > >> >> If we do writeback via some sort of OUT_FB_ID property on plane/crtc, > >> >> we will need the same sort of format information so userspace knows > >> >> what output formats (and modifiers) are supported. So re-using the > >> >> same blob property layout (and userspace parsing) seems useful. > >> > > >> >I'd definitely be cool with this. It doesn't make our lives any easier > >> >or more difficult in terms of parsing, and it also avoids a dep on new > >> >libdrm API/ABI, which is always nice. If anyone types this up, I'll > >> >happily port the Weston WIP. > >> > > >> >Cheers, > >> >Daniel > >> > >> Okay. Sounds like we have consensus. I'm working on it now. I think like > >> Rob > >> said on IRC, a good amount of it will be reusable from GET_PLANE2. I'm a > >> bit new > >> to the binary blob props, so if anyone has a strong opinion on how it > >> should > >> look, please speak now. Otherwise, I'll just wire up something. > >> > >Can you tell me the status of this? > >I'm looking at adding tiled scanout to imx-drm and just want to know if > >it's worth to hold my breath for the reworked patch to arrive. > > > >Regards, > >Lucas > > > > The agreement was to use a blob property instead of GET_PLANE2. I wrote the > patches: > https://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=blobifier > > However, my test vehicle, kmscube has broken on i915, so I haven't yet been > able > to test and therefore I didn't send them yet. Daniel said he'd try to wire it > up > to weston next week. > > I can go back and try to make it work with legacy kmscube as well, unless > someone wants to fix kmscube/i915/i965 first? > Thanks. I'll pull those in an try to get things working with imx-drm and kmscube. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v6 02/12] drm/atomic: Add support for custom scaling mode properties, v2
Op 02-05-17 om 11:44 schreef Daniel Vetter:
> On Mon, May 01, 2017 at 03:37:54PM +0200, Maarten Lankhorst wrote:
>> Some connectors may not allow all scaling mode properties, this function
>> will allow
>> creating the scaling mode property with only the supported subset. It also
>> wires up
>> this state for atomic.
>>
>> This will make it possible to convert i915 connectors to atomic.
>>
>> Changes since v1:
>> - Add DRM_MODE_PROP_ENUM flag to drm_property_create
>> - Use the correct index in drm_property_add_enum.
>> - Add DocBook for function (Sean Paul).
>> - Warn if less than 2 valid scaling modes are passed.
>> - Remove level of indent. (Sean Paul)
>>
>> Signed-off-by: Maarten Lankhorst
>> ---
>> drivers/gpu/drm/drm_atomic.c| 4 +++
>> drivers/gpu/drm/drm_connector.c | 58
>> +
>> include/drm/drm_connector.h | 10 +++
>> 3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 77bb36e956db..c7f91dcebbe9 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1125,6 +1125,8 @@ int drm_atomic_connector_set_property(struct
>> drm_connector *connector,
>> state->link_status = val;
>> } else if (property == config->aspect_ratio_property) {
>> state->picture_aspect_ratio = val;
>> +} else if (property == connector->scaling_mode_property) {
>> +state->scaling_mode = val;
> Can't we still handle mode_config->scaling_mode_property as fallback?
> Seems a lot more consistent to me ...
>
>> } else if (connector->funcs->atomic_set_property) {
>> return connector->funcs->atomic_set_property(connector,
>> state, property, val);
>> @@ -1203,6 +1205,8 @@ drm_atomic_connector_get_property(struct drm_connector
>> *connector,
>> *val = state->link_status;
>> } else if (property == config->aspect_ratio_property) {
>> *val = state->picture_aspect_ratio;
>> +} else if (property == connector->scaling_mode_property) {
>> +*val = state->scaling_mode;
>> } else if (connector->funcs->atomic_get_property) {
>> return connector->funcs->atomic_get_property(connector,
>> state, property, val);
>> diff --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c
>> index 9f847615ac74..b3912f2e48c6 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -961,6 +961,64 @@ int drm_mode_create_scaling_mode_property(struct
>> drm_device *dev)
>> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>>
>> /**
>> + * drm_mode_connector_attach_scaling_mode_property - attach atomic scaling
>> mode property
>> + * @connector: connector to attach scaling mode property on.
>> + * @scaling_mode_mask: or'ed mask of BIT(DRM_MODE_SCALE_\*).
>> + *
>> + * This is used to add support for scaling mode to atomic drivers.
>> + * The scaling mode will be set to &struct
>> drm_connector_state->picture_aspect_ratio
> s/->/./ to get a real link
>
>> + * and can be used from &struct drm_connector_helper_funcs->atomic_check
>> for validation.
> Same here, plus needs &.
>
> Please check the html output when typing docs ...
>
> Also please link to drm_mode_create_scaling_mode_property() and from the
> kerneldoc of that to this one here.
Ah right, thanks.
>
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int drm_mode_connector_attach_scaling_mode_property(struct drm_connector
>> *connector,
>> +u32 scaling_mode_mask)
> Usual prefix is just drm_connector_ (yes I know we're not consistent here,
> yet).
>
> With those nits: Reviewed-by: Daniel Vetter
>
Will rename. :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 1/7] drm/atomic: initial support for asynchronous plane update
Hi, On 2 May 2017 at 09:10, Daniel Vetter wrote: > On Fri, Apr 21, 2017 at 03:41:10PM -0300, Gustavo Padovan wrote: >> Do you know which hw would that be? I don't know. Maybe we should just >> don't care for now and see how drivers will solve this to then extract >> helpers like we did for atomic nonblocking commits. > > i915 is one. The only way to do true async updates is throught the CS flip > command with a special bit set, and that one only works for the primary > plane. All other updates are synced to vblank, i.e. the hw will keep > scanning out the old address. > > But I checked the current code, it's no better than what you've done. > > More special is iirc rockchip (or some other arm-soc display ip), which on top > also has an iommu which falls over if the mapping disappears right away. > Easy solution is to not allow fb changes (but that kinda sucks), more > involved is delaying the fb cleanup into a worker (but since we don't > rate-limit this is tricky too ...). Most ARM display IP these days has it (notable exception is VC4 and I think also i.MX?), and will do that. Exynos also has no way to update the base address during scanout, and will not like it if it starts smashing into faults from its IOMMU. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/5] drm/msm: Reuse dma_fence_release.
On Wed, Apr 12, 2017 at 3:12 PM, Eric Anholt wrote:
> If we follow the typical pattern of the base class being the first
> member, we can use the default dma_fence_free function.
>
> Signed-off-by: Eric Anholt
> Cc: Rob Clark
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
Reviewed-by: Rob Clark
(go ahead and push via drm-misc with the rest of the series, if you
haven't already)
> ---
> drivers/gpu/drm/msm/msm_fence.c | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
> index 3f299c537b77..a2f89bac9c16 100644
> --- a/drivers/gpu/drm/msm/msm_fence.c
> +++ b/drivers/gpu/drm/msm/msm_fence.c
> @@ -99,8 +99,8 @@ void msm_update_fence(struct msm_fence_context *fctx,
> uint32_t fence)
> }
>
> struct msm_fence {
> - struct msm_fence_context *fctx;
> struct dma_fence base;
> + struct msm_fence_context *fctx;
> };
>
> static inline struct msm_fence *to_msm_fence(struct dma_fence *fence)
> @@ -130,19 +130,13 @@ static bool msm_fence_signaled(struct dma_fence *fence)
> return fence_completed(f->fctx, f->base.seqno);
> }
>
> -static void msm_fence_release(struct dma_fence *fence)
> -{
> - struct msm_fence *f = to_msm_fence(fence);
> - kfree_rcu(f, base.rcu);
> -}
> -
> static const struct dma_fence_ops msm_fence_ops = {
> .get_driver_name = msm_fence_get_driver_name,
> .get_timeline_name = msm_fence_get_timeline_name,
> .enable_signaling = msm_fence_enable_signaling,
> .signaled = msm_fence_signaled,
> .wait = dma_fence_default_wait,
> - .release = msm_fence_release,
> + .release = dma_fence_free,
> };
>
> struct dma_fence *
> --
> 2.11.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/5] drm/msm: Expose our reservation object when exporting a dmabuf.
On Wed, Apr 12, 2017 at 3:11 PM, Eric Anholt wrote:
> Without this, polling on the dma-buf (and presumably other devices
> synchronizing against our rendering) would return immediately, even
> while the BO was busy.
>
> Signed-off-by: Eric Anholt
> Reviewed-by: Daniel Vetter
> Cc: sta...@vger.kernel.org
> Cc: Rob Clark
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
Reviewed-by: Rob Clark
(go ahead and push via drm-misc with the rest of the series, if you
haven't already)
> ---
> drivers/gpu/drm/msm/msm_drv.c | 1 +
> drivers/gpu/drm/msm/msm_drv.h | 1 +
> drivers/gpu/drm/msm/msm_gem_prime.c | 7 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 9208e67be453..fe728a134e16 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -829,6 +829,7 @@ static struct drm_driver msm_driver = {
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_export = drm_gem_prime_export,
> .gem_prime_import = drm_gem_prime_import,
> + .gem_prime_res_obj = msm_gem_prime_res_obj,
> .gem_prime_pin = msm_gem_prime_pin,
> .gem_prime_unpin= msm_gem_prime_unpin,
> .gem_prime_get_sg_table = msm_gem_prime_get_sg_table,
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index b885c3d5ae4d..5e67fa66d483 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -223,6 +223,7 @@ struct sg_table *msm_gem_prime_get_sg_table(struct
> drm_gem_object *obj);
> void *msm_gem_prime_vmap(struct drm_gem_object *obj);
> void msm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct
> *vma);
> +struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj);
> struct drm_gem_object *msm_gem_prime_import_sg_table(struct drm_device *dev,
> struct dma_buf_attachment *attach, struct sg_table *sg);
> int msm_gem_prime_pin(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c
> b/drivers/gpu/drm/msm/msm_gem_prime.c
> index 60bb290700ce..13403c6da6c7 100644
> --- a/drivers/gpu/drm/msm/msm_gem_prime.c
> +++ b/drivers/gpu/drm/msm/msm_gem_prime.c
> @@ -70,3 +70,10 @@ void msm_gem_prime_unpin(struct drm_gem_object *obj)
> if (!obj->import_attach)
> msm_gem_put_pages(obj);
> }
> +
> +struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj)
> +{
> + struct msm_gem_object *msm_obj = to_msm_bo(obj);
> +
> + return msm_obj->resv;
> +}
> --
> 2.11.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v6 05/12] drm/i915: Add plumbing for digital connector state, v3.
On Mon, May 01, 2017 at 03:37:57PM +0200, Maarten Lankhorst wrote:
> Some atomic properties are common between the various kinds of
> connectors, for example a lot of them use panel fitting mode.
> It makes sense to put a lot of it in a common place, so each
> connector can use it while they're being converted.
>
> Implement the properties required for the connectors:
> - scaling mode property
> - force audio property
> - broadcast rgb
> - aspect ratio
>
> While at it, make clear that intel_digital_connector_atomic_get_property
> is a hack that has to be removed when all connector properties
> are converted to atomic.
>
> Changes since v1:
> - Scaling mode and aspect ratio are partly handled in core now.
> Changes since v2:
> - Split out the scaling mode / aspect ratio changes to a preparation
> patch.
> - Use mode_changed for panel fitter, changes to this property
> are checked by fastset.
> - Allowed_scaling_modes is removed, handled through core now.
>
> Signed-off-by: Maarten Lankhorst
Patches 3-5: Reviewed-by: Daniel Vetter
Pls ping Dave for an Ack for getting the first 2 landed through drm-intel
(they'll still show up in the first pull request right after -rc1, so all
good imo).
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 131
> +--
> drivers/gpu/drm/i915/intel_display.c | 14 +++-
> drivers/gpu/drm/i915/intel_drv.h | 23 ++
> 3 files changed, 159 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 50fb1f76cc5f..182909f266f5 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -36,7 +36,7 @@
> #include "intel_drv.h"
>
> /**
> - * intel_connector_atomic_get_property - fetch connector property value
> + * intel_connector_atomic_get_property - fetch legacy connector property
> value
> * @connector: connector to fetch property for
> * @state: state containing the property value
> * @property: property to look up
> @@ -45,12 +45,14 @@
> * The DRM core does not store shadow copies of properties for
> * atomic-capable drivers. This entrypoint is used to fetch
> * the current value of a driver-specific connector property.
> + *
> + * This is a intermediary solution until all connectors are
> + * converted to support full atomic properties.
> */
> -int
> -intel_connector_atomic_get_property(struct drm_connector *connector,
> - const struct drm_connector_state *state,
> - struct drm_property *property,
> - uint64_t *val)
> +int intel_connector_atomic_get_property(struct drm_connector *connector,
> + const struct drm_connector_state *state,
> + struct drm_property *property,
> + uint64_t *val)
> {
> int i;
>
> @@ -73,7 +75,122 @@ intel_connector_atomic_get_property(struct drm_connector
> *connector,
> return -EINVAL;
> }
>
> -/*
> +/**
> + * intel_digital_connector_atomic_get_property - hook for
> connector->atomic_get_property.
> + * @connector: Connector to get the property for.
> + * @state: Connector state to retrieve the property from.
> + * @property: Property to retrieve.
> + * @val: Return value for the property.
> + *
> + * Returns the atomic property value for a digital connector.
> + */
> +int intel_digital_connector_atomic_get_property(struct drm_connector
> *connector,
> + const struct
> drm_connector_state *state,
> + struct drm_property *property,
> + uint64_t *val)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_digital_connector_state *intel_conn_state =
> + to_intel_digital_connector_state(state);
> +
> + if (property == dev_priv->force_audio_property)
> + *val = intel_conn_state->force_audio;
> + else if (property == dev_priv->broadcast_rgb_property)
> + *val = intel_conn_state->broadcast_rgb;
> + else {
> + DRM_DEBUG_ATOMIC("Unknown property %s\n", property->name);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * intel_digital_connector_atomic_set_property - hook for
> connector->atomic_set_property.
> + * @connector: Connector to set the property for.
> + * @state: Connector state to set the property on.
> + * @property: Property to set.
> + * @val: New value for the property.
> + *
> + * Sets the atomic property value for a digital connector.
> + */
> +int intel_digital_connector_atomic_set_property(struct drm_connector
> *connector,
> + struct drm_connector_state
> *state,
> +
Re: [PATCH v6 01/12] drm/atomic: Handle picture_aspect_ratio in atomic core
On Mon, May 01, 2017 at 03:37:53PM +0200, Maarten Lankhorst wrote:
> This is only used in i915, which had used its own non-taomic way to
> deal with the picture aspect ratio. Move selected aspect_ratio to
> atomic state and use the atomic state in the affected i915 connectors.
>
> Signed-off-by: Maarten Lankhorst
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic.c | 4
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_hdmi.c | 18 +++---
> drivers/gpu/drm/i915/intel_sdvo.c | 23 +++
> include/drm/drm_connector.h | 10 ++
> 5 files changed, 20 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 30229ab719c0..77bb36e956db 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1123,6 +1123,8 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
>*/
> if (state->link_status != DRM_LINK_STATUS_GOOD)
> state->link_status = val;
> + } else if (property == config->aspect_ratio_property) {
> + state->picture_aspect_ratio = val;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -1199,6 +1201,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = state->tv.hue;
> } else if (property == config->link_status_property) {
> *val = state->link_status;
> + } else if (property == config->aspect_ratio_property) {
> + *val = state->picture_aspect_ratio;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 54f3ff840812..d38fed78500b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -877,7 +877,6 @@ struct intel_hdmi {
> bool has_audio;
> enum hdmi_force_audio force_audio;
> bool rgb_quant_range_selectable;
> - enum hdmi_picture_aspect aspect_ratio;
> struct intel_connector *attached_connector;
> void (*write_infoframe)(struct drm_encoder *encoder,
> const struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 52f0b2d5fad2..58d690393b29 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1408,7 +1408,7 @@ bool intel_hdmi_compute_config(struct intel_encoder
> *encoder,
> }
>
> /* Set user selected PAR to incoming mode's member */
> - adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
> + adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio;
>
> pipe_config->lane_count = 4;
>
> @@ -1654,19 +1654,7 @@ intel_hdmi_set_property(struct drm_connector
> *connector,
> }
>
> if (property == connector->dev->mode_config.aspect_ratio_property) {
> - switch (val) {
> - case DRM_MODE_PICTURE_ASPECT_NONE:
> - intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> - break;
> - case DRM_MODE_PICTURE_ASPECT_4_3:
> - intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_4_3;
> - break;
> - case DRM_MODE_PICTURE_ASPECT_16_9:
> - intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_16_9;
> - break;
> - default:
> - return -EINVAL;
> - }
> + connector->state->picture_aspect_ratio = val;
> goto done;
> }
>
> @@ -1828,7 +1816,7 @@ intel_hdmi_add_properties(struct intel_hdmi
> *intel_hdmi, struct drm_connector *c
> intel_attach_broadcast_rgb_property(connector);
> intel_hdmi->color_range_auto = true;
> intel_attach_aspect_ratio_property(connector);
> - intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> + connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> }
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index 816a6f5a3fd9..ef6fa87b2f8a 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -107,11 +107,6 @@ struct intel_sdvo {
> bool color_range_auto;
>
> /**
> - * HDMI user specified aspect ratio
> - */
> - enum hdmi_picture_aspect aspect_ratio;
> -
> - /**
>* This is set if we're going to treat the device as TV-out.
>*
>* While we have these nice friendly flags for output types that ought
> @@ -1186,7 +11
Re: [Intel-gfx] [PATCH v6 02/12] drm/atomic: Add support for custom scaling mode properties, v2
On Mon, May 01, 2017 at 03:37:54PM +0200, Maarten Lankhorst wrote:
> Some connectors may not allow all scaling mode properties, this function will
> allow
> creating the scaling mode property with only the supported subset. It also
> wires up
> this state for atomic.
>
> This will make it possible to convert i915 connectors to atomic.
>
> Changes since v1:
> - Add DRM_MODE_PROP_ENUM flag to drm_property_create
> - Use the correct index in drm_property_add_enum.
> - Add DocBook for function (Sean Paul).
> - Warn if less than 2 valid scaling modes are passed.
> - Remove level of indent. (Sean Paul)
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/drm_atomic.c| 4 +++
> drivers/gpu/drm/drm_connector.c | 58
> +
> include/drm/drm_connector.h | 10 +++
> 3 files changed, 72 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 77bb36e956db..c7f91dcebbe9 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1125,6 +1125,8 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->link_status = val;
> } else if (property == config->aspect_ratio_property) {
> state->picture_aspect_ratio = val;
> + } else if (property == connector->scaling_mode_property) {
> + state->scaling_mode = val;
Can't we still handle mode_config->scaling_mode_property as fallback?
Seems a lot more consistent to me ...
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -1203,6 +1205,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = state->link_status;
> } else if (property == config->aspect_ratio_property) {
> *val = state->picture_aspect_ratio;
> + } else if (property == connector->scaling_mode_property) {
> + *val = state->scaling_mode;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 9f847615ac74..b3912f2e48c6 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -961,6 +961,64 @@ int drm_mode_create_scaling_mode_property(struct
> drm_device *dev)
> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>
> /**
> + * drm_mode_connector_attach_scaling_mode_property - attach atomic scaling
> mode property
> + * @connector: connector to attach scaling mode property on.
> + * @scaling_mode_mask: or'ed mask of BIT(DRM_MODE_SCALE_\*).
> + *
> + * This is used to add support for scaling mode to atomic drivers.
> + * The scaling mode will be set to &struct
> drm_connector_state->picture_aspect_ratio
s/->/./ to get a real link
> + * and can be used from &struct drm_connector_helper_funcs->atomic_check for
> validation.
Same here, plus needs &.
Please check the html output when typing docs ...
Also please link to drm_mode_create_scaling_mode_property() and from the
kerneldoc of that to this one here.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_mode_connector_attach_scaling_mode_property(struct drm_connector
> *connector,
> + u32 scaling_mode_mask)
Usual prefix is just drm_connector_ (yes I know we're not consistent here,
yet).
With those nits: Reviewed-by: Daniel Vetter
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_property *scaling_mode_property;
> + int i, j = 0;
> + const unsigned valid_scaling_mode_mask =
> + (1U << ARRAY_SIZE(drm_scaling_mode_enum_list)) - 1;
> +
> + if (WARN_ON(hweight32(scaling_mode_mask) < 2 ||
> + scaling_mode_mask & ~valid_scaling_mode_mask))
> + return -EINVAL;
> +
> + scaling_mode_property =
> + drm_property_create(dev, DRM_MODE_PROP_ENUM, "scaling mode",
> + hweight32(scaling_mode_mask));
> +
> + if (!scaling_mode_property)
> + return -ENOMEM;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_scaling_mode_enum_list); i++) {
> + int ret;
> +
> + if (!(BIT(i) & scaling_mode_mask))
> + continue;
> +
> + ret = drm_property_add_enum(scaling_mode_property, j++,
> + drm_scaling_mode_enum_list[i].type,
> + drm_scaling_mode_enum_list[i].name);
> +
> + if (ret) {
> + drm_property_destroy(dev, scaling_mode_property);
> +
> + return ret;
> + }
> + }
> +
> + drm_obj
Re: linux-next: build failure after merge of the drm-misc tree
On Tue, May 2, 2017 at 10:55 AM, Arnd Bergmann wrote: > On Tue, May 2, 2017 at 10:41 AM, Stephen Rothwell > wrote: >> Hi Daniel, >> >> On Tue, 2 May 2017 10:25:18 +0200 Daniel Vetter wrote: >>> >>> Since this is an all-new driver it might be best to stagger the pull >>> requests and merge the new tee subsystem (or whatever it is) after drm? >>> >>> Not sure what to best do here ... >> >> This will merge via Dave, so Dave just needs to let Linus know that a >> fix up is needed when this merges with the arm-soc stuff in Linus' tree. > > The TEE subsystem is currently on a separate branch by itself in arm-soc, > so we could easily delay that until DRM is in, and point out the resolution > here. Dave is somewhere without mail, but I chatted with him quickly and he agrees this seems simplest. drm-next main pull should go out somewhere tomorrow he said. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 3/6] drm: Plane YCbCr to RGB conversion related properties
On 05/02/17 11:33, Daniel Vetter wrote:
> On Fri, Apr 21, 2017 at 12:51:14PM +0300, Jyri Sarha wrote:
>> Add standard properties to control YCbCr to RGB conversion in DRM
>> planes. The created properties are stored to drm_plane object to allow
>> different set of supported conversion modes for different planes on
>> the device. For simplicity the related property blobs for CSC matrix
>> and YCbCr preoffsets are also stored in the same place. The blob
>> contents are defined in the uapi/drm/drm_mode.h header.
>>
>> Signed-off-by: Jyri Sarha
>
> Just to make sure there's no surprises: We need the userspace for this
> too. -modesetting (for Xv maybe), some wayland compositor (weston, ozone,
> whatever you feel like) or drm_hwcomposer.
>
> But yeah might be good to bikeshed the uabi first a bit more and get at
> least some agreement on that.
>
In the first phase I have been using kms++ [1]. And when/if we have an
agreement about the API, I will push my patches there.
With X, wayland, and other compositors, we would in the end need some
video player supporting the different YCbCr encodings according to the
video stream. This sounds like a relatively big task, but surely I
volunteer to assist, when we get there.
Cheer,
Jyri
[1] https://github.com/tomba/kmsxx/
> Thanks, Daniel
>> ---
>> drivers/gpu/drm/drm_atomic.c| 26 +++
>> drivers/gpu/drm/drm_atomic_helper.c | 9 +++
>> drivers/gpu/drm/drm_color_mgmt.c| 136
>> +++-
>> drivers/gpu/drm/drm_plane.c | 10 +++
>> include/drm/drm_color_mgmt.h| 23 ++
>> include/drm/drm_plane.h | 10 +++
>> include/uapi/drm/drm_mode.h | 12
>> 7 files changed, 223 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index f881319..d1512aa 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -732,6 +732,8 @@ int drm_atomic_plane_set_property(struct drm_plane
>> *plane,
>> {
>> struct drm_device *dev = plane->dev;
>> struct drm_mode_config *config = &dev->mode_config;
>> +int ret;
>> +bool dummy;
>>
>> if (property == config->prop_fb_id) {
>> struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val);
>> @@ -774,6 +776,22 @@ int drm_atomic_plane_set_property(struct drm_plane
>> *plane,
>> state->rotation = val;
>> } else if (property == plane->zpos_property) {
>> state->zpos = val;
>> +} else if (property == plane->ycbcr_to_rgb_mode_property) {
>> +state->ycbcr_to_rgb_mode = val;
>> +} else if (property == plane->ycbcr_to_rgb_csc_property) {
>> +ret = drm_atomic_replace_property_blob_from_id(dev,
>> +&state->ycbcr_to_rgb_csc,
>> +val,
>> +sizeof(struct drm_ycbcr_to_rgb_csc),
>> +&dummy);
>> +return ret;
>> +} else if (property == plane->ycbcr_to_rgb_preoffset_property) {
>> +ret = drm_atomic_replace_property_blob_from_id(dev,
>> +&state->ycbcr_to_rgb_preoffset,
>> +val,
>> +sizeof(struct drm_ycbcr_to_rgb_preoffset),
>> +&dummy);
>> +return ret;
>> } else if (plane->funcs->atomic_set_property) {
>> return plane->funcs->atomic_set_property(plane, state,
>> property, val);
>> @@ -834,6 +852,14 @@ int drm_atomic_plane_set_property(struct drm_plane
>> *plane,
>> *val = state->rotation;
>> } else if (property == plane->zpos_property) {
>> *val = state->zpos;
>> +} else if (property == plane->ycbcr_to_rgb_mode_property) {
>> +*val = state->ycbcr_to_rgb_mode;
>> +} else if (property == plane->ycbcr_to_rgb_csc_property) {
>> +*val = state->ycbcr_to_rgb_csc ?
>> +state->ycbcr_to_rgb_csc->base.id : 0;
>> +} else if (property == plane->ycbcr_to_rgb_preoffset_property) {
>> +*val = state->ycbcr_to_rgb_preoffset ?
>> +state->ycbcr_to_rgb_preoffset->base.id : 0;
>> } else if (plane->funcs->atomic_get_property) {
>> return plane->funcs->atomic_get_property(plane, state,
>> property, val);
>> } else {
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index c3994b4..89fd826 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -3196,6 +3196,12 @@ void __drm_atomic_helper_plane_duplicate_state(struct
>> drm_plane *plane,
>> {
>> memcpy(state, plane->state, sizeof(*state));
>>
>> +if (state->ycbcr_to_rgb_csc)
>> +drm_property_blob_get(state->ycbcr_to_rgb_csc);
>> +
>> +if (state->ycbcr_to_rgb_preoffset)
>> +drm_pro
Re: [PATCH v4 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
On Fri, Apr 28, 2017 at 03:42:22PM -0700, Eric Anholt wrote:
> The FBDEV initialization would throw an error in dmesg, when we just
> want to silently not initialize fbdev on a V3D-only VC4 instance.
>
> Signed-off-by: Eric Anholt
With the commit message updated that passing num_connector is the bug that
throws the error (and not that we set up a no-op fbdev):
Reviewed-by: Daniel Vetter
Still kinda hoping for a follow-up to entirely get rid fo num_connector in
the fbdev init funcs.
-Daniel
> ---
> drivers/gpu/drm/vc4/vc4_kms.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index ad7925a9e0ea..237a504f11f0 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -230,10 +230,12 @@ int vc4_kms_load(struct drm_device *dev)
>
> drm_mode_config_reset(dev);
>
> - vc4->fbdev = drm_fbdev_cma_init(dev, 32,
> - dev->mode_config.num_connector);
> - if (IS_ERR(vc4->fbdev))
> - vc4->fbdev = NULL;
> + if (dev->mode_config.num_connector) {
> + vc4->fbdev = drm_fbdev_cma_init(dev, 32,
> + dev->mode_config.num_connector);
> + if (IS_ERR(vc4->fbdev))
> + vc4->fbdev = NULL;
> + }
>
> drm_kms_helper_poll_init(dev);
>
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: fix doc to use new name for commit types
On Thu, Apr 27, 2017 at 11:35:06AM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > > Use "non-blocking" and "blocking" instead of old names. > > Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index cfeda5f..5a3c9c0 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1070,8 +1070,8 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables); > * > * Note that @pre_swap is needed since the point where we block for fences > moves > * around depending upon whether an atomic commit is blocking or > - * non-blocking. For async commit all waiting needs to happen after > - * drm_atomic_helper_swap_state() is called, but for synchronous commits we > want > + * non-blocking. For non-blocking commit all waiting needs to happen after > + * drm_atomic_helper_swap_state() is called, but for blocking commits we want > * to wait **before** we do anything that can't be easily rolled back. That > is > * before we call drm_atomic_helper_swap_state(). > * > -- > 2.9.3 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCHv2 2/3] drm/prime: Introduce drm_gem_prime_import_platform
On Wed, Apr 26, 2017 at 02:12:29PM -0700, Laura Abbott wrote:
> The existing drm_gem_prime_import function uses the underlying
> struct device of a drm_device for attaching to a dma_buf. Some drivers
> (notably vgem) may not have an underlying device structure. Offer
> an alternate function to attach using a platform device associated
> with drm_device.
>
> Signed-off-by: Laura Abbott
> ---
> New approach for v2
This conflicts with patches already in drm-misc.git/drm-misc-next. Can you
pls rebase (and then also pls add the missing kerneldoc for the new
drm_gem_prime_import_platform function, plus links between the two any
why they exists/when to use each)?
Noticed this when I tried to apply them.
Pls keep Chris' r-b on the patch when resending.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_prime.c | 23 ---
> include/drm/drmP.h | 5 +
> 2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 25aa455..5486248 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -601,8 +601,9 @@ EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
> * This is the implementation of the gem_prime_import functions for GEM
> drivers
> * using the PRIME helpers.
> */
> -struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> - struct dma_buf *dma_buf)
> +static struct drm_gem_object *__drm_gem_prime_import(struct drm_device *dev,
> + struct dma_buf *dma_buf,
> + struct device *attach_dev)
> {
> struct dma_buf_attachment *attach;
> struct sg_table *sgt;
> @@ -624,7 +625,7 @@ struct drm_gem_object *drm_gem_prime_import(struct
> drm_device *dev,
> if (!dev->driver->gem_prime_import_sg_table)
> return ERR_PTR(-EINVAL);
>
> - attach = dma_buf_attach(dma_buf, dev->dev);
> + attach = dma_buf_attach(dma_buf, attach_dev);
> if (IS_ERR(attach))
> return ERR_CAST(attach);
>
> @@ -654,8 +655,24 @@ struct drm_gem_object *drm_gem_prime_import(struct
> drm_device *dev,
>
> return ERR_PTR(ret);
> }
> +
> +struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + return __drm_gem_prime_import(dev, dma_buf, dev->dev);
> +}
> EXPORT_SYMBOL(drm_gem_prime_import);
>
> +struct drm_gem_object *drm_gem_prime_import_platform(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + if (WARN_ON_ONCE(!dev->platformdev))
> + return NULL;
> +
> + return __drm_gem_prime_import(dev, dma_buf, &dev->platformdev->dev);
> +}
> +EXPORT_SYMBOL(drm_gem_prime_import_platform);
> +
> /**
> * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
> * @dev: dev to export the buffer from
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 6105c05..f4ac30f 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -769,6 +769,11 @@ extern int drm_gem_prime_handle_to_fd(struct drm_device
> *dev,
> int *prime_fd);
> extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> struct dma_buf *dma_buf);
> +
> +extern struct drm_gem_object *drm_gem_prime_import_platform(
> + struct drm_device *dev,
> + struct dma_buf *dma_buf);
> +
> extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> struct drm_file *file_priv, int prime_fd, uint32_t *handle);
> struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
> --
> 2.7.4
>
--
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: [PATCH] drm/msm: fix splat when userspace is killed with pending atomic update
On Fri, Apr 28, 2017 at 8:05 PM, Rob Clark wrote: > The ->preclose() hook is a good place to block for pending atomic > updates. We can't do this in ->postclose(), as it needs to happen > before drm_fb_release(). Otherwise, since we have already swapped > state (in the case of a non-blocking atomic update), this means that > the plane_state->fb will be released and cleared before we wait for > fences from the atomic-commit wq. > > There are probably more complex solutions possible. But since already > scheduled atomic update, possibly blocking on already scheduled gpu/etc > fences, will complete eventually (assuming nothing catches fire), so > the sanest thing seems to be just block until already scheduled atomic > updates complete before tearing things down. > > Fixes: > >WARNING: CPU: 1 PID: 69 at ../drivers/gpu/drm/drm_atomic_helper.c:1061 > drm_atomic_helper_wait_for_fences+0xe0/0xf8 >Modules linked in: > >CPU: 1 PID: 69 Comm: kworker/1:1 Tainted: GW 4.11.0-rc8+ > #1187 >Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) >Workqueue: events drm_mode_rmfb_work_fn >task: ffc036560d00 task.stack: ffc03655 >PC is at drm_atomic_helper_wait_for_fences+0xe0/0xf8 >LR is at complete_commit.isra.1+0x44/0x1c0 >pc : [] lr : [] pstate: 2145 >sp : ffc036553b60 >x29: ffc036553b60 x28: ffc0264e6a00 >x27: ffc035659000 x26: >x25: ffc0240e8000 x24: 0038 >x23: x22: ff800858f200 >x21: ffc0240e8000 x20: ffc02f56a800 >x19: x18: >x17: x16: >x15: x14: ffc00a192700 >x13: 0004 x12: >x11: ff80089a1690 x10: 08f0 >x9 : ffc036553b20 x8 : ffc036561650 >x7 : ffc03fe6cb40 x6 : >x5 : 0001 x4 : 0002 >x3 : ffc035659000 x2 : ffc0240e8c80 >x1 : x0 : ffc02adbe588 > >---[ end trace 13aeec77c3fb55e2 ]--- >Call trace: >Exception stack(0xffc036553990 to 0xffc036553ac0) >3980: 0080 >39a0: ffc036553b60 ff80084f6040 4ff0 0038 >39c0: ffc0365539d0 ff800857e098 ffc036553a00 ff800857e1b0 >39e0: ffc036553a10 ff800857c554 ffc0365e8400 ffc0365e8400 >3a00: ffc036553a20 ff8008103358 0001aad7 ff800851b72c >3a20: ffc036553a50 ff80080e9228 ffc02adbe588 >3a40: ffc0240e8c80 ffc035659000 0002 0001 >3a60: ffc03fe6cb40 ffc036561650 ffc036553b20 >3a80: 08f0 ff80089a1690 0004 >3aa0: ffc00a192700 >[] drm_atomic_helper_wait_for_fences+0xe0/0xf8 >[] complete_commit.isra.1+0x44/0x1c0 >[] msm_atomic_commit+0x32c/0x350 >[] drm_atomic_commit+0x50/0x60 >[] drm_atomic_remove_fb+0x158/0x250 >[] drm_framebuffer_remove+0x50/0x158 >[] drm_mode_rmfb_work_fn+0x40/0x58 >[] process_one_work+0x1d0/0x378 >[] worker_thread+0x244/0x488 >[] kthread+0xfc/0x128 >[] ret_from_fork+0x10/0x50 > > Reported-by: Stanimir Varbanov > Signed-off-by: Rob Clark > --- > The hunk that removes the comment about ->preclose() included in this > patch to challenge the assumption that ->preclose() shouldn't exist ;-) And I'm going to challenge your patch here. Both fences and framebuffers and atomic commits are refcounted. If you go boom on them when userspace closes the fd, you have a refcount bug. We don't fix those by flusing stuff :-) Please add a pair of get/put() calls at the right place instead. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the drm-misc tree
On Tue, May 2, 2017 at 10:41 AM, Stephen Rothwell wrote: > Hi Daniel, > > On Tue, 2 May 2017 10:25:18 +0200 Daniel Vetter wrote: >> >> Since this is an all-new driver it might be best to stagger the pull >> requests and merge the new tee subsystem (or whatever it is) after drm? >> >> Not sure what to best do here ... > > This will merge via Dave, so Dave just needs to let Linus know that a > fix up is needed when this merges with the arm-soc stuff in Linus' tree. The TEE subsystem is currently on a separate branch by itself in arm-soc, so we could easily delay that until DRM is in, and point out the resolution here. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback
On Wed, Apr 26, 2017 at 11:48:34AM +0100, Jose Abreu wrote:
> Some crtc's may have restrictions in the mode they can display. In
> this patch a new callback (crtc->mode_valid()) is introduced that
> is called at the same stage of connector->mode_valid() callback.
>
> This shall be implemented if the crtc has some sort of restriction
> so that we don't probe modes that will fail in the commit() stage.
> For example: A given crtc may be responsible to set a clock value.
> If the clock can not produce all the values for the available
> modes then this callback can be used to restrict the number of
> probbed modes to only the ones that can be displayed.
>
> If the crtc does not implement the callback then the behaviour will
> remain the same. Also, for a given set of crtcs that can be bound to
> the connector, if at least one can display the mode then the mode
> will be probbed.
>
> Signed-off-by: Jose Abreu
> Cc: Carlos Palminha
> Cc: Alexey Brodkin
> Cc: Ville Syrjälä
> Cc: Daniel Vetter
> Cc: Dave Airlie
Not sure this is useful, since you still have to duplicate the exact same
check into your ->mode_fixup hook. That seems to make things even more
confusing.
Also this doesn't update the various kerneldoc comments. For the existing
hooks. Since this topic causes so much confusion, I don't think a
half-solution will help, but has some good potential to make things worse.
-Daniel
> ---
> drivers/gpu/drm/drm_probe_helper.c | 44
>
> include/drm/drm_modeset_helper_vtables.h | 26 +++
> 2 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 1b0c14a..61eac30 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -80,6 +80,46 @@
> return MODE_OK;
> }
>
> +static enum drm_mode_status drm_mode_validate_connector_crtc(
> + struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + const struct drm_crtc_helper_funcs *crtc_funcs = NULL;
> + enum drm_mode_status mode_status = MODE_ERROR;
> + struct drm_device *dev = connector->dev;
> + struct drm_encoder *encoder;
> + struct drm_crtc *crtc;
> + bool callback_found = false;
> + int i;
> +
> + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> + encoder = drm_encoder_find(dev, connector->encoder_ids[i]);
> +
> + if (!encoder)
> + continue;
> +
> + drm_for_each_crtc(crtc, dev) {
> + crtc_funcs = crtc->helper_private;
> +
> + if (!drm_encoder_crtc_ok(encoder, crtc))
> + continue;
> + if (!crtc_funcs || !crtc_funcs->mode_valid)
> + continue;
> +
> + /* MODE_OK=0 and default mode_status=MODE_ERROR=-1
> + * so if at least one crtc accepts the mode we get
> + * MODE_OK */
> + mode_status &= crtc_funcs->mode_valid(crtc, mode);
> + callback_found |= true;
> + }
> + }
> +
> + /* We can reach here without calling mode_valid if there is no
> + * registered crtc with this callback, lets return MODE_OK in this
> + * case */
> + return callback_found ? mode_status : MODE_OK;
> +}
> +
> static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
> {
> struct drm_cmdline_mode *cmdline_mode;
> @@ -431,6 +471,10 @@ int drm_helper_probe_single_connector_modes(struct
> drm_connector *connector,
> if (mode->status == MODE_OK && connector_funcs->mode_valid)
> mode->status = connector_funcs->mode_valid(connector,
> mode);
> +
> + if (mode->status == MODE_OK)
> + mode->status = drm_mode_validate_connector_crtc(
> + connector, mode);
> }
>
> prune:
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index c01c328..59fffba 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -106,6 +106,32 @@ struct drm_crtc_helper_funcs {
> void (*commit)(struct drm_crtc *crtc);
>
> /**
> + * @mode_valid:
> + *
> + * This callback should be implemented if the crtc has some sort of
> + * restriction in the modes it can display. For example, a given crtc
> + * may be responsible to set a clock value. If the clock can not
> + * produce all the values for the available modes then this callback
> + * can be used to restrict the number of probbed modes to only the ones
> + * that can be displayed.
> + *
> + * This is directly called at the same stage of connector->mode_valid
&
Re: linux-next: build failure after merge of the drm-misc tree
Hi Daniel, On Tue, 2 May 2017 10:25:18 +0200 Daniel Vetter wrote: > > Since this is an all-new driver it might be best to stagger the pull > requests and merge the new tee subsystem (or whatever it is) after drm? > > Not sure what to best do here ... This will merge via Dave, so Dave just needs to let Linus know that a fix up is needed when this merges with the arm-soc stuff in Linus' tree. -- Cheers, Stephen Rothwell ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Probing modes and validating them
On Fri, Apr 21, 2017 at 02:29:12PM +0100, Jose Abreu wrote:
> ++ Daniel
>
> ++ Dave
>
>
> On 21-04-2017 13:59, Jose Abreu wrote:
> > Hi All,
> >
> >
> > Is there any callback available, at the crtc level, that can
> > validate the probed modes before making them available for
> > userspace? I mean: I know that there is connector->mode_valid(),
> > but I couldn't find any equivalent crtc->mode_valid(). I found
> > mode_fixup() but this is called after giving the mode to
> > userspace, which is not what I need.
> >
> > For reference here is the situation I'm facing:
> >
> > ARCPGU is a DRM driver that uses ADV7511 as connector/bridge. The
> > PGU encodes raw video into a stream that ADV can understands and
> > in order to do this it needs a pixel clock. Currently this pixel
> > clock value is fixed, so technically arcpgu driver supports only
> > one video mode. There is a patch currently on discussion that
> > adds more supported frequencies for arcpgu, but there are still
> > some frequencies that will not be supported. This is NOT a
> > limitation in ADV7511, so it doesn't make sense to limit the
> > available modes in adv, we could instead limit the modes in the
> > crtc level (i.e. the pgu).
> >
> > In order to know if a mode can be displayed or not it is as
> > simple as call clk_round_rate() and check if the returned
> > frequency is the same. But in order to do this I need some sort
> > of callback that is called at the crtc level and before
> > delivering modes to userspace.
> >
> > I believe this would be a good addition to arcpgu because this
> > way we wouldn't "fool" userspace by delivering modes that will
> > fail in atomic check. The user may still specify manually a mode,
> > this will still fail in atomic check, but the difference is that
> > this mode will not be in the probed list.
This is a faq, and your patch isn't the solution. Adding John (who made a
different spin on this, differently broken).
I guess I to write a todo.rst entry about this ...
-Daniel
> >
> >
> > Best regards,
> >
> > Jose Miguel Abreu
> >
>
> With this simple patch I could fix my problem, what do you think?
> Is this ok to be added? I guess some connectors may not have the
> crtc linked at probbing stage but this is handled.
>
> From 252b7cb5adeaf16be95988d17468eea2895b Mon Sep 17 00:00:00
> 2001
> Message-Id:
> <252b7cb5adeaf16be95988d17468eea2895b.1492781127.git.joab...@synopsys.com>
> From: Jose Abreu
> Date: Fri, 21 Apr 2017 14:24:16 +0100
> Subject: [PATCH] drm: Introduce crtc->mode_valid() callback
>
> Introduce a new crtc callback which validates the probbed modes,
> just like connector->mode_valid() does.
>
> Signed-off-by: Jose Abreu
> ---
> drivers/gpu/drm/arc/arcpgu_crtc.c| 14 ++
> drivers/gpu/drm/drm_probe_helper.c | 12
> include/drm/drm_modeset_helper_vtables.h | 3 +++
> 3 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c
> b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index 7130b04..e43e8d6 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -63,6 +63,19 @@ static void arc_pgu_set_pxl_fmt(struct
> drm_crtc *crtc)
> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> };
>
> +enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> +struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> +long rate, clk_rate = mode->clock * 1000;
> +
> +rate = clk_round_rate(arcpgu->clk, clk_rate);
> +if (rate != clk_rate)
> +return MODE_NOCLOCK;
> +
> +return MODE_OK;
> +}
> +
> static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc)
> {
> struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> @@ -157,6 +170,7 @@ static void arc_pgu_crtc_atomic_begin(struct
> drm_crtc *crtc,
> }
>
> static const struct drm_crtc_helper_funcs
> arc_pgu_crtc_helper_funcs = {
> +.mode_valid= arc_pgu_crtc_mode_valid,
> .mode_set= drm_helper_crtc_mode_set,
> .mode_set_base= drm_helper_crtc_mode_set_base,
> .mode_set_nofb= arc_pgu_crtc_mode_set_nofb,
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index cf8f012..6c9af88 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -233,6 +233,8 @@ int
> drm_helper_probe_single_connector_modes(struct drm_connector
> *connector,
> struct drm_display_mode *mode;
> const struct drm_connector_helper_funcs *connector_funcs =
> connector->helper_private;
> +struct drm_crtc *crtc = NULL;
> +const struct drm_crtc_helper_funcs *crtc_funcs = NULL;
> int count = 0;
> int mode_flags = 0;
> bool verbose_prune = true;
> @@ -242,6 +244,13 @@ int
> drm_helper_probe_single_connector_modes(struct drm_connector
> *connector,
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
Re: [PATCH RFC 3/6] drm: Plane YCbCr to RGB conversion related properties
On Fri, Apr 21, 2017 at 12:51:14PM +0300, Jyri Sarha wrote:
> Add standard properties to control YCbCr to RGB conversion in DRM
> planes. The created properties are stored to drm_plane object to allow
> different set of supported conversion modes for different planes on
> the device. For simplicity the related property blobs for CSC matrix
> and YCbCr preoffsets are also stored in the same place. The blob
> contents are defined in the uapi/drm/drm_mode.h header.
>
> Signed-off-by: Jyri Sarha
Just to make sure there's no surprises: We need the userspace for this
too. -modesetting (for Xv maybe), some wayland compositor (weston, ozone,
whatever you feel like) or drm_hwcomposer.
But yeah might be good to bikeshed the uabi first a bit more and get at
least some agreement on that.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_atomic.c| 26 +++
> drivers/gpu/drm/drm_atomic_helper.c | 9 +++
> drivers/gpu/drm/drm_color_mgmt.c| 136
> +++-
> drivers/gpu/drm/drm_plane.c | 10 +++
> include/drm/drm_color_mgmt.h| 23 ++
> include/drm/drm_plane.h | 10 +++
> include/uapi/drm/drm_mode.h | 12
> 7 files changed, 223 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f881319..d1512aa 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -732,6 +732,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
> {
> struct drm_device *dev = plane->dev;
> struct drm_mode_config *config = &dev->mode_config;
> + int ret;
> + bool dummy;
>
> if (property == config->prop_fb_id) {
> struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val);
> @@ -774,6 +776,22 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> state->rotation = val;
> } else if (property == plane->zpos_property) {
> state->zpos = val;
> + } else if (property == plane->ycbcr_to_rgb_mode_property) {
> + state->ycbcr_to_rgb_mode = val;
> + } else if (property == plane->ycbcr_to_rgb_csc_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + &state->ycbcr_to_rgb_csc,
> + val,
> + sizeof(struct drm_ycbcr_to_rgb_csc),
> + &dummy);
> + return ret;
> + } else if (property == plane->ycbcr_to_rgb_preoffset_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + &state->ycbcr_to_rgb_preoffset,
> + val,
> + sizeof(struct drm_ycbcr_to_rgb_preoffset),
> + &dummy);
> + return ret;
> } else if (plane->funcs->atomic_set_property) {
> return plane->funcs->atomic_set_property(plane, state,
> property, val);
> @@ -834,6 +852,14 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> *val = state->rotation;
> } else if (property == plane->zpos_property) {
> *val = state->zpos;
> + } else if (property == plane->ycbcr_to_rgb_mode_property) {
> + *val = state->ycbcr_to_rgb_mode;
> + } else if (property == plane->ycbcr_to_rgb_csc_property) {
> + *val = state->ycbcr_to_rgb_csc ?
> + state->ycbcr_to_rgb_csc->base.id : 0;
> + } else if (property == plane->ycbcr_to_rgb_preoffset_property) {
> + *val = state->ycbcr_to_rgb_preoffset ?
> + state->ycbcr_to_rgb_preoffset->base.id : 0;
> } else if (plane->funcs->atomic_get_property) {
> return plane->funcs->atomic_get_property(plane, state,
> property, val);
> } else {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index c3994b4..89fd826 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3196,6 +3196,12 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> {
> memcpy(state, plane->state, sizeof(*state));
>
> + if (state->ycbcr_to_rgb_csc)
> + drm_property_blob_get(state->ycbcr_to_rgb_csc);
> +
> + if (state->ycbcr_to_rgb_preoffset)
> + drm_property_blob_get(state->ycbcr_to_rgb_preoffset);
> +
> if (state->fb)
> drm_framebuffer_get(state->fb);
>
> @@ -3236,6 +3242,9 @@ struct drm_plane_state *
> */
> void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
> {
> + drm_property_blob_put(state->ycbcr_to_rgb_csc);
> + drm_property_blob_put(state->ycbcr_to_rgb_preoffset);
> +
> if (state->fb)
> drm_framebuffer_put(state->fb);
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
> b/drivers/gpu/dr
Re: [PATCH RFC 2/6] drm: Make drm_atomic_replace_property_blob_from_id() more generic
On Fri, Apr 21, 2017 at 02:47:44PM +0300, Laurent Pinchart wrote:
> Hi Jyri,
>
> Thank you for the patch.
>
> On Friday 21 Apr 2017 12:51:13 Jyri Sarha wrote:
> > Change drm_atomic_replace_property_blob_from_id()'s first parameter
> > from drm_crtc to drm_device, so that the function can be used for other
> > drm_mode_objects too.
> >
> > Signed-off-by: Jyri Sarha
>
> Reviewed-by: Laurent Pinchart
First two patches applied for 4.13, thanks.
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 9b892af..f881319 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -425,7 +425,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct
> > drm_crtc_state *state, }
> >
> > static int
> > -drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
> > +drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> > struct drm_property_blob **blob,
> > uint64_t blob_id,
> > ssize_t expected_size,
> > @@ -434,7 +434,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct
> > drm_crtc_state *state, struct drm_property_blob *new_blob = NULL;
> >
> > if (blob_id != 0) {
> > - new_blob = drm_property_lookup_blob(crtc->dev, blob_id);
> > + new_blob = drm_property_lookup_blob(dev, blob_id);
> > if (new_blob == NULL)
> > return -EINVAL;
> >
> > @@ -483,7 +483,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> > drm_property_blob_put(mode);
> > return ret;
> > } else if (property == config->degamma_lut_property) {
> > - ret = drm_atomic_replace_property_blob_from_id(crtc,
> > + ret = drm_atomic_replace_property_blob_from_id(dev,
> > &state->degamma_lut,
> > val,
> > -1,
> > @@ -491,7 +491,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> > state->color_mgmt_changed |= replaced;
> > return ret;
> > } else if (property == config->ctm_property) {
> > - ret = drm_atomic_replace_property_blob_from_id(crtc,
> > + ret = drm_atomic_replace_property_blob_from_id(dev,
> > &state->ctm,
> > val,
> > sizeof(struct drm_color_ctm),
> > @@ -499,7 +499,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> > state->color_mgmt_changed |= replaced;
> > return ret;
> > } else if (property == config->gamma_lut_property) {
> > - ret = drm_atomic_replace_property_blob_from_id(crtc,
> > + ret = drm_atomic_replace_property_blob_from_id(dev,
> > &state->gamma_lut,
> > val,
> > -1,
>
> --
> Regards,
>
> Laurent Pinchart
>
--
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: linux-next: build failure after merge of the drm-misc tree
On Mon, Apr 24, 2017 at 11:25:12AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> On Fri, 21 Apr 2017 12:10:14 +1000 Stephen Rothwell
> wrote:
> >
> > After merging the drm-misc tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > drivers/tee/tee_shm.c:87:2: error: unknown field 'kmap_atomic' specified in
> > initializer
> > .kmap_atomic = tee_shm_op_kmap_atomic,
> > ^
> > drivers/tee/tee_shm.c:87:17: error: initialization from incompatible
> > pointer type [-Werror=incompatible-pointer-types]
> > .kmap_atomic = tee_shm_op_kmap_atomic,
> > ^
> > drivers/tee/tee_shm.c:87:17: note: (near initialization for
> > 'tee_shm_dma_buf_ops.begin_cpu_access')
> > drivers/tee/tee_shm.c:88:2: error: unknown field 'kmap' specified in
> > initializer
> > .kmap = tee_shm_op_kmap,
> > ^
> > drivers/tee/tee_shm.c:88:10: error: initialization from incompatible
> > pointer type [-Werror=incompatible-pointer-types]
> > .kmap = tee_shm_op_kmap,
> > ^
> > drivers/tee/tee_shm.c:88:10: note: (near initialization for
> > 'tee_shm_dma_buf_ops.end_cpu_access')
> >
> > Caused by commit
> >
> > f9b67f0014cb ("dma-buf: Rename dma-ops to prevent conflict with
> > kunmap_atomic macro")
> >
> > interacting with commit
> >
> > 967c9cca2cc5 ("tee: generic TEE subsystem")
> >
> > from the arm-soc tree.
> >
> > I applied the following merge fix patch for today:
> >
> > From: Stephen Rothwell
> > Date: Fri, 21 Apr 2017 12:06:32 +1000
> > Subject: [PATCH] tee: merge fix for dma-ops field name changes
> >
> > Signed-off-by: Stephen Rothwell
> > ---
> > drivers/tee/tee_shm.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > index 0be1e3e93bee..4e14c9c9cb1c 100644
> > --- a/drivers/tee/tee_shm.c
> > +++ b/drivers/tee/tee_shm.c
> > @@ -84,8 +84,8 @@ static struct dma_buf_ops tee_shm_dma_buf_ops = {
> > .map_dma_buf = tee_shm_op_map_dma_buf,
> > .unmap_dma_buf = tee_shm_op_unmap_dma_buf,
> > .release = tee_shm_op_release,
> > - .kmap_atomic = tee_shm_op_kmap_atomic,
> > - .kmap = tee_shm_op_kmap,
> > + .map_atomic = tee_shm_op_kmap_atomic,
> > + .map = tee_shm_op_kmap,
> > .mmap = tee_shm_op_mmap,
> > };
Since this is an all-new driver it might be best to stagger the pull
requests and merge the new tee subsystem (or whatever it is) after drm?
Not sure what to best do here ...
-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: [PATCH] drm: trivial documentation fix to drm_for_each_connector_iter
On Thu, Apr 20, 2017 at 09:38:19PM -0300, Gabriel Krisman Bertazi wrote:
> While reading drm_for_each_connector_iter, I noticed a mention to
> drm_connector_begin which doesn't exist. It should be
> drm_connector_get.
>
> Signed-off-by: Gabriel Krisman Bertazi
Fixes: b982dab1e66d ("drm: Rename connector list iterator API")
Reviewed-by: Daniel Vetter
> ---
> include/drm/drm_connector.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4eeda120e46d..ff428aa0953a 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1031,7 +1031,7 @@ void drm_connector_list_iter_end(struct
> drm_connector_list_iter *iter);
> *
> * Note that @connector is only valid within the list body, if you want to
> use
> * @connector after calling drm_connector_list_iter_end() then you need to
> grab
> - * your own reference first using drm_connector_begin().
> + * your own reference first using drm_connector_get().
> */
> #define drm_for_each_connector_iter(connector, iter) \
> while ((connector = drm_connector_list_iter_next(iter)))
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4.1 01/9] drm/atomic: Handle aspect ratio and scaling mode in core, v2.
Op 02-05-17 om 10:12 schreef Daniel Vetter:
> On Mon, Apr 24, 2017 at 02:01:05PM +0200, Maarten Lankhorst wrote:
>> On 19-04-17 17:43, Daniel Vetter wrote:
>>> On Thu, Apr 13, 2017 at 11:15:37AM +0200, Maarten Lankhorst wrote:
This is required to for i915 to convert connector properties to atomic.
Changes since v1:
- Add docbook info. (danvet)
- Change picture_aspect_ratio to enum hdmi_picture_aspect.
Signed-off-by: Maarten Lankhorst
Cc: dri-devel@lists.freedesktop.org
Acked-by: Dave Airlie
---
drivers/gpu/drm/drm_atomic.c | 8
include/drm/drm_connector.h | 16
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 30229ab719c0..25ea6f797a54 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct
drm_connector *connector,
*/
if (state->link_status != DRM_LINK_STATUS_GOOD)
state->link_status = val;
+ } else if (property == config->aspect_ratio_property) {
+ state->picture_aspect_ratio = val;
+ } else if (property == config->scaling_mode_property) {
+ state->scaling_mode = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct
drm_connector *connector,
*val = state->tv.hue;
} else if (property == config->link_status_property) {
*val = state->link_status;
+ } else if (property == config->aspect_ratio_property) {
+ *val = state->picture_aspect_ratio;
+ } else if (property == config->scaling_mode_property) {
+ *val = state->scaling_mode;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4eeda120e46d..5b50bc2db6fb 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -25,6 +25,7 @@
#include
#include
+#include
#include
#include
@@ -326,6 +327,21 @@ struct drm_connector_state {
struct drm_atomic_state *state;
struct drm_tv_connector_state tv;
+
+ /**
+ * @picture_aspect_ratio: Connector property to control the
+ * HDMI infoframe aspect ratio setting.
+ *
+ * The DRM_MODE_PICTURE_ASPECT_* values much match the
>>> I think the above will upset sphinx and spew a warning, you need
>>> ASPECT_\* or something like that. Or just spell them out and enumerate
>>> them all. Fixed either way this is
>> I checked and make htmldocs worked just fine, but I'll quote the *.
> I meant inspecting the visual output ... Without the * escaped iirc you
> can end up with the entire paragraph bold.
> -Daniel
Yeah either way worked though for the html documentation, at least when I
checked it locally. But it didn't complain either option so v5 and v6 use \*.
~Maarten
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
On Mon, Apr 24, 2017 at 10:26:45AM -0400, Alex Deucher wrote: > On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt wrote: > > Daniel Vetter writes: > > > >> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt wrote: > >>> Daniel Vetter writes: > On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt wrote: > > The FBDEV initialization would throw an error in dmesg, when we just > > want to silently not initialize fbdev on a V3D-only VC4 instance. > > > > Signed-off-by: Eric Anholt > > Hm, this shouldn't be an error really, you might want to hotplug more > connectors later on. What exactly complains? > >>> > >>> drm_fb_helper_init() throws an error if the passed in connector count is > >>> 0, so drm_fb_cma_helper() printks an error. > >> > >> Oh, _that_ thing. The error in there is correct, but (almost) everyone > >> gets this parameter wrong. This isn't the max number of connectors the > >> fb helper will light up, but just the max number of connectors _per_ > >> crtc when driving in hw clone mode. There's two problems with that: > >> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each > >> active connector > >> - I mentioned that everyone gets this wrong? > >> > >> If you're moderately bored it'd be great to nuke the max_connector > >> argument from drm_fb_helper_init, and hard-code it to 1 (with a big > >> comment explaining that this needs to be changed, probably with > >> dynamic reallocation, once someone gets around to implementing hw > >> clone mode). > >> > >> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a > >> TODO.rst entry would be great in that case. > > > > If I'm driving a GPU with no display subsystem at all, it seems like I > > shouldn't initialize fbdev for it, right? > > That's what we do for radeon/amdgpu on hw without display blocks. Yeah I got slightly distracted with the error :-) Still might make sense to put this logic into the helpers, if there's no crtc then don't bother initializing the fbcon. But imo the better cleanup is getting rid of the connectors parameter, since most drivers get it wrong, then respin the vc4 patch to explain what it's really doing (it's not about the error really, it's about not registering and fbdev that's not doing anything). -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: [PATCH libdrm v2] Header: Add rotation property fields
On Mon, Apr 24, 2017 at 01:51:39PM +0100, Emil Velikov wrote: > On 18 April 2017 at 23:16, Daniel Vetter wrote: > > On Tue, Apr 18, 2017 at 8:33 PM, Kristian Høgsberg > > wrote: > As far as I understand the property mechanism, the numeric values > aren't actually ABI. The string names of the enum values are the ABI > and you're supposed to parse the enum info and discover the numerical > values. For example: > https://git.collabora.com/cgit/user/daniels/weston.git/tree/libweston/compositor-drm.c?h=wip/2017-03/atomic-v10-WIP#n570 > > >>> Note sure I agree, yet then again my kernel experience is less than yours. > >>> Do check the following commit and feel free to search the ML thread > >>> that inspired it. > >> > >> I haven't worked much with the property mechanism tbh, but I know > >> Daniel (Stone) jumped through all those hoops to avoid hard-coding the > >> enum values. > > > > In theory, this is correct and is how it's supposed to be done. In > > practice, for a bunch of properties at least, we deal with the reality > > of userspace being lazy and assuming that the enum values match with > > the encode they send over the wire and tears result if we ever chance > > that. I still think that going through the strings should be the > > better way, since it makes it much easier to add/remove certain enum > > values, depending upon what the hw/driverr combo can pull off. > > > > The story is different for the properties themselves, there you > > definitely need to make the name->prop id lookup, and you also need to > > do that mapping for each object separately (because the value range is > > attached to the prop, for uabi reasons, but might need to be > > per-object). > > Daniel, this that mean that we're OK with moving DRM_ROTATE* [+others] > to a ABI header? With a big comment, but yeah this is special. -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: [PATCH v4.1 01/9] drm/atomic: Handle aspect ratio and scaling mode in core, v2.
On Mon, Apr 24, 2017 at 02:01:05PM +0200, Maarten Lankhorst wrote:
> On 19-04-17 17:43, Daniel Vetter wrote:
> > On Thu, Apr 13, 2017 at 11:15:37AM +0200, Maarten Lankhorst wrote:
> >> This is required to for i915 to convert connector properties to atomic.
> >>
> >> Changes since v1:
> >> - Add docbook info. (danvet)
> >> - Change picture_aspect_ratio to enum hdmi_picture_aspect.
> >>
> >> Signed-off-by: Maarten Lankhorst
> >> Cc: dri-devel@lists.freedesktop.org
> >> Acked-by: Dave Airlie
> >> ---
> >> drivers/gpu/drm/drm_atomic.c | 8
> >> include/drm/drm_connector.h | 16
> >> 2 files changed, 24 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >> index 30229ab719c0..25ea6f797a54 100644
> >> --- a/drivers/gpu/drm/drm_atomic.c
> >> +++ b/drivers/gpu/drm/drm_atomic.c
> >> @@ -1123,6 +1123,10 @@ int drm_atomic_connector_set_property(struct
> >> drm_connector *connector,
> >> */
> >>if (state->link_status != DRM_LINK_STATUS_GOOD)
> >>state->link_status = val;
> >> + } else if (property == config->aspect_ratio_property) {
> >> + state->picture_aspect_ratio = val;
> >> + } else if (property == config->scaling_mode_property) {
> >> + state->scaling_mode = val;
> >>} else if (connector->funcs->atomic_set_property) {
> >>return connector->funcs->atomic_set_property(connector,
> >>state, property, val);
> >> @@ -1199,6 +1203,10 @@ drm_atomic_connector_get_property(struct
> >> drm_connector *connector,
> >>*val = state->tv.hue;
> >>} else if (property == config->link_status_property) {
> >>*val = state->link_status;
> >> + } else if (property == config->aspect_ratio_property) {
> >> + *val = state->picture_aspect_ratio;
> >> + } else if (property == config->scaling_mode_property) {
> >> + *val = state->scaling_mode;
> >>} else if (connector->funcs->atomic_get_property) {
> >>return connector->funcs->atomic_get_property(connector,
> >>state, property, val);
> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> >> index 4eeda120e46d..5b50bc2db6fb 100644
> >> --- a/include/drm/drm_connector.h
> >> +++ b/include/drm/drm_connector.h
> >> @@ -25,6 +25,7 @@
> >>
> >> #include
> >> #include
> >> +#include
> >> #include
> >>
> >> #include
> >> @@ -326,6 +327,21 @@ struct drm_connector_state {
> >>struct drm_atomic_state *state;
> >>
> >>struct drm_tv_connector_state tv;
> >> +
> >> + /**
> >> + * @picture_aspect_ratio: Connector property to control the
> >> + * HDMI infoframe aspect ratio setting.
> >> + *
> >> + * The DRM_MODE_PICTURE_ASPECT_* values much match the
> > I think the above will upset sphinx and spew a warning, you need
> > ASPECT_\* or something like that. Or just spell them out and enumerate
> > them all. Fixed either way this is
> I checked and make htmldocs worked just fine, but I'll quote the *.
I meant inspecting the visual output ... Without the * escaped iirc you
can end up with the entire paragraph bold.
-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: [RFC 1/7] drm/atomic: initial support for asynchronous plane update
On Fri, Apr 21, 2017 at 03:41:10PM -0300, Gustavo Padovan wrote: > 2017-04-11 Daniel Vetter : > > > On Mon, Apr 10, 2017 at 12:55:33PM -0700, Eric Anholt wrote: > > > Gustavo Padovan writes: > > > > > > > From: Gustavo Padovan > > Some async cursor updates are not 100% async, as in the hw might still > > scan out the old buffer until the next vblank. Could/should we handle this > > in core? Or are we going to shrug this off with "meh, you asked for > > tearing, you got tearing"? > > Do you know which hw would that be? I don't know. Maybe we should just > don't care for now and see how drivers will solve this to then extract > helpers like we did for atomic nonblocking commits. i915 is one. The only way to do true async updates is throught the CS flip command with a special bit set, and that one only works for the primary plane. All other updates are synced to vblank, i.e. the hw will keep scanning out the old address. But I checked the current code, it's no better than what you've done. More special is iirc rockchip (or some other arm-soc display ip), which on top also has an iommu which falls over if the mapping disappears right away. Easy solution is to not allow fb changes (but that kinda sucks), more involved is delaying the fb cleanup into a worker (but since we don't rate-limit this is tricky too ...). Maybe just go with a FIXME comment here? -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: Proposal for RandR version 1.6, Leases and EDID-based output grabs
On Fri, 28 Apr 2017 15:03:17 -0700
Keith Packard wrote:
> Pekka Paalanen writes:
>
> > IMHO, if nothing is providing a picture intended for the HMD, the HMD
> > should be off. There is no use in showing an arbitrary image that does
> > not look right, is there?
>
> Well, if the HMD is being worn and the application crashes, then
> what you want is something that keeps responding to your motion so you
> can get the HMD off without falling or running into stuff...
Hi,
I was under the impression that if you have a VR application running on
the HMD, it necessarily also implies that you have a VR compositor
running, which means that there is always some process providing a valid
image to the HMD. (At least in end-user setups.)
Here the assumption is that the application may crash or stall, but the
VR compositor never will. Obviously that's not exactly true, but if we
design for also the VR compositor crashing or stalling, then we have a
requirement for a tertiary process that is essentially just like the
VR compositor. And so the turtles pile up... which is also why I don't
think a backup plan for the VR compositor is necessary.
Or did you have an idea of something extremely simple that could still
provide a reasonable image for the HMD even when the compositors have
crashed and the GPU is frozen?
However, your original question was:
> * What should we do with an HMD which is in the database but for which
>no application is installed on the host?
I assumed that means there are no VR apps nor a VR compositor that
could handle the HMD. In that case I think the HMD should be always off.
> But, yeah, in general, if you don't have an HMD application running, the
> HMD can't usefully show anything from a bare window system. The trick is
> to make sure existing desktop applications don't try to use it by mistake.
Sure.
> > even if the database was just a bunch of files, you still need code to
> > access it, and probably something to ensure the entries are
> > well-formed, so that everyone will agree on how to parse them. One
> > should probably decide whether the database will only answer the
> > question "is it a HMD?" or can it provide other information as well.
>
> Yup, we need a spec for the files that is reasonably sane, and also
> extensible so that if we want to add new data, we can. I discussed this
> with Eric Anholt at breakfast this morning and we came up with a couple
> of ideas:
>
> 1) A directory full of files, each file can contain one or more monitor
> entries
>
> 2) Monitors should be identified (at a minimum) using the EDID
> Manufacturer ID and Manufacturer product code. I can imagine
> also allowing the use of a serial number and/or date code if we end
> up using this for more stuff later.
>
> 3) Window system independent. We obviously need this for X, but
> other window systems should share the same data.
>
> 4) Use an existing format. Both of us would rather use JSON, but
> there's already XML in the DRM world, so that might make
> sense. These are functionally equivalent, but XML syntax is rough on
> the eyes.
>
> 5) Allow multiple definitions in each file, but allow for multiple
> files too.
>
> Here's a JSON-formatted example:
>
> {
> "monitors": [
> {
> "Manufacturer" : "LGD",
> "Product" : 437,
> "desktop" : true
> }
> ]
> "copyright" : "Copyright © 2017, Keith Packard",
> "license" : "GPLv3"
> }
>
> One can imagine the same done in XML, but I'm too lazy to type that
> here. In any case, as you can see above, I've added a "desktop" field;
> if false, the monitor would be hidden to 'normal' desktop applications
> and only made visible to others.
I presume that if "desktop" is set to "true", it implies that the HMD
is capable of showing a simple 2D canvas in stereo without any special
rendering and with the default video mode. That is, creating a sort of
a virtual 2D monitor. That would be nice.
> Questions:
>
> Q) Where should the directory live.
>
> A) /usr/share/monitors for distribution-provided files, /etc/monitors
> for application-provided files.
>
> Q) How about RandR protocol.
>
> A) I'm thinking of just creating a new randr request like
> RRGetOutputInfo but which will return even "hidden" outputs with
> non-spoofed 'connection' value.
>
> Q) What file names should the entries use.
>
> A) How about just the manufacturer and product of the first entry?
>
> Q) Ranges of product ids?
>
> A) Yeah, we might want this to avoid a lot of duplicate entries.
>
FWIW, it all sounds good to me!
I don't really have an opinion on XML vs. JSON, I'm just happy it's not
another ad hoc format.
Thanks,
pq
pgph6oemqubvP.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.
