Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix command parser to validate multiple register access with the same command.
The patchset LGTM and works well with beignet. The 80%+ performance regression
issue in darktable also has been fixed
after this patchset applied and enable the atomic in L3 at beignet side. So,
Reviewed-by: Zhigang Gong
Thanks,
Zhigang Gong.
> -Original Message-
> From: Francisco Jerez [mailto:curroje...@riseup.net]
> Sent: Friday, May 29, 2015 9:44 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Ville Syrjälä; Zhigang Gong; Brad Volkin
> Subject: [PATCH 1/3] drm/i915: Fix command parser to validate multiple
> register access with the same command.
>
> Until now the software command checker assumed that commands could read
> or write at most a single register per packet. This is not necessarily the
> case,
> MI_LOAD_REGISTER_IMM expects a variable-length list of offset/value pairs
> and writes them in sequence. The previous code would only check whether
> the first entry was valid, effectively allowing userspace to write
> unrestricted
> registers of the MMIO space by sending a multi-register write with a legal
> first
> register, with potential security implications on Gen6 and 7 hardware.
>
> Fix it by extending the drm_i915_cmd_descriptor table to represent
> multi-register access and making validate_cmd() iterate for all register
> offsets
> present in the command packet.
>
> Signed-off-by: Francisco Jerez
> ---
> drivers/gpu/drm/i915/i915_cmd_parser.c | 74
> --
> drivers/gpu/drm/i915/i915_drv.h| 5 +++
> 2 files changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
> b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 61ae8ff..c4a5f73 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -123,7 +123,7 @@ static const struct drm_i915_cmd_descriptor
> common_cmds[] = {
> CMD( MI_SEMAPHORE_MBOX,SMI, !F, 0xFF,
> R ),
> CMD( MI_STORE_DWORD_INDEX, SMI, !F, 0xFF,
> R ),
> CMD( MI_LOAD_REGISTER_IMM(1), SMI, !F, 0xFF,
> W,
> - .reg = { .offset = 1, .mask = 0x007C } ),
> + .reg = { .offset = 1, .mask = 0x007C, .step = 2 }),
> CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF,
> W | B,
> .reg = { .offset = 1, .mask = 0x007C },
> .bits = {{
> @@ -939,7 +939,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs
> *ring)
>
> static bool check_cmd(const struct intel_engine_cs *ring,
> const struct drm_i915_cmd_descriptor *desc,
> - const u32 *cmd,
> + const u32 *cmd, u32 length,
> const bool is_master,
> bool *oacontrol_set)
> {
> @@ -955,38 +955,49 @@ static bool check_cmd(const struct intel_engine_cs
> *ring,
> }
>
> if (desc->flags & CMD_DESC_REGISTER) {
> - u32 reg_addr = cmd[desc->reg.offset] & desc->reg.mask;
> -
> /*
> - * OACONTROL requires some special handling for writes. We
> - * want to make sure that any batch which enables OA also
> - * disables it before the end of the batch. The goal is to
> - * prevent one process from snooping on the perf data from
> - * another process. To do that, we need to check the value
> - * that will be written to the register. Hence, limit
> - * OACONTROL writes to only MI_LOAD_REGISTER_IMM
> commands.
> + * Get the distance between individual register offset
> + * fields if the command can perform more than one
> + * access at a time.
>*/
> - if (reg_addr == OACONTROL) {
> - if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
> - DRM_DEBUG_DRIVER("CMD: Rejected LRM to
> OACONTROL\n");
> - return false;
> + const u32 step = desc->reg.step ? desc->reg.step : length;
> + u32 offset;
> +
> + for (offset = desc->reg.offset; offset < length;
> + offset += step) {
> + const u32 reg_addr = cmd[offset] & desc->reg.mask;
> +
> + /*
> + * OACONTROL requires some special handling for
> + * writes. We want to make sure that any batch which
> + * enables OA also disables it before the end of the
> + * batch. The goal is to prevent one process from
> + * snooping
Re: [Intel-gfx] [Beignet] Haswell issues
Andi, Thanks for your comments and that makes sense for me, just check kernel version is not an ideal method for those unofficial kernels with back porting patches. Then we have the following open questions in my mind: How do we check whether the i915 KMD support secure batch buffer execution if the batch buffer pass the cmd parser check under full-ppgtt mode in UMD? How do we check whether the i915 KMD support secure batch buffer execution with aliasing ppgtt after the merging of the patch "drm/i915: Arm cmd parser with aliasing ppgtt only" in UMD? CC to Daniel, do you have any suggestion here? Thanks, Zhigang Gong. > -Original Message- > From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of > Andi Kleen > Sent: Thursday, April 30, 2015 12:19 PM > To: beig...@lists.freedesktop.org > Cc: zhigang.g...@linux.intel.com > Subject: Re: [Beignet] Haswell issues > > "Zhigang Gong" writes: > >> but I don't have the hardware to try it. > > We will do some testing on this. And once we get the exact version, I > > will submit a new patch to give some warn if the user is using an > > unsupported kernel. As now we have better solution than patch kernel > > manually, I will change the document to suggest user to upgrade to a > supported kernel rather than patch kernel manually. > > It would be better to add something to the kernel interface that can be probed > without relying on the kernel version. The problem with checking versions is > that it'll break if someone backports the fixes to older kernels, which is not > uncommon. > > -Andi > > -- > a...@linux.intel.com -- Speaking for myself only > > ___ > Beignet mailing list > beig...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/beignet ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
oach: 2. About the MAP_FIXED approach. If my understanding is correct, this is a intermediate solution towards FULL SVM function. From OCL's view, it is usefull for the 2.0's. There are three types of SVM options. Even for the most basic "Coarse-Grained buffer SVM", something like MAP_FIXED is useful. Becuase it needs to pass a linked list object to the OCL kernel directly. If we can use MAP_FIXED to map the bo to the VM address which is used by host, then we can access such a linked list easily on OCL kernel side, otherwise, we may need some tricks to do extra address adjustment. So this feature is useful for Beignet before we get full SVM support. My conclusion is: * The 1st is a totally *passive* requirement and need to be applied to each buffer binding. And it's better to be transparent to userspace. * The 2nd is an *active* requirement and need the kernel to provide SVM like interface to the user space. And only those SVM buffers (for the first 2 OCL SVM options) need to call the new interfaces explicitly. Thanks, Zhigang Gong. > > Thanks, > Jesse > > ___ > Beignet mailing list > beig...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/beignet ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
My only concern is about the following macros:
> +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33
> +#define LOCAL_I915_PARAM_EU_TOTAL34
How about to just use the definitons in the kernel header file?
For an example:
#include
#ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL
//Put all the code into this block.
#endif
Then we can avoid put the same definitions in different files,
and we can avoid unecessary testing on an old kernel which doesn't
have this kernel interface.
For all the other part, it LGTM.
Reviewed-by: Zhigang Gong
Thanks,
Zhigang Gong.
On Thu, Mar 12, 2015 at 05:26:25PM -0700, jeff.mc...@intel.com wrote:
> From: Jeff McGee
>
> New test core_getparams consists of 2 subtests, each one testing
> the ability of userspace to query the correct value of a GT config
> attribute: subslice total or EU total. drm/i915 implementation of
> these queries is required for Cherryview and Gen9+ devices (non-
> simulated).
>
> v2: Duplicate small amount of new libdrm functionality to avoid
> bumping libdrm version requirement (Daniel). Convert some
> igt_asserts to the appropriate comparison variants. Add a
> test description.
> v3: Actually use the LOCAL GETPARAM defines. Otherwise can't build
> against older libdrm as intended by v2.
>
> For: VIZ-4636
> Signed-off-by: Jeff McGee
> ---
> tests/.gitignore | 1 +
> tests/Makefile.sources | 1 +
> tests/core_getparams.c | 167
> +
> 3 files changed, 169 insertions(+)
> create mode 100644 tests/core_getparams.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 426cc67..c742308 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -1,6 +1,7 @@
> # Please keep sorted alphabetically
> core_get_client_auth
> core_getclient
> +core_getparams
> core_getstats
> core_getversion
> drm_import_export
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 51e8376..999c8f8 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \
>
> TESTS_progs_M = \
> core_get_client_auth \
> + core_getparams \
> drv_suspend \
> drv_hangman \
> gem_bad_reloc \
> diff --git a/tests/core_getparams.c b/tests/core_getparams.c
> new file mode 100644
> index 000..2855d06
> --- /dev/null
> +++ b/tests/core_getparams.c
> @@ -0,0 +1,167 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Jeff McGee
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include "drmtest.h"
> +#include "intel_chipset.h"
> +#include "intel_bufmgr.h"
> +
> +IGT_TEST_DESCRIPTION("Tests the export of parameters via
> DRM_IOCTL_I915_GETPARAM\n");
> +
> +int drm_fd;
> +int devid;
> +
> +static void
> +init(void)
> +{
> + drm_fd = drm_open_any();
> + devid = intel_get_drm_devid(drm_fd);
> +}
> +
> +static void
> +deinit(void)
> +{
> + close(drm_fd);
> +}
> +
> +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33
> +#define LOCAL_I915_PARAM_EU_TOTAL34
> +
> +static int
> +getparam(int param, int *value)
> +{
> + drm_i915_getparam_t gp;
> + int ret;
> +
> + memset(&gp, 0, sizeof(gp));
> + gp.value = value;
> + gp.param = param;
> + ret = drmIoctl(drm_fd, DRM_IOCTL_I915_GETPARAM, &gp);
> + if (ret)
> + return -e
Re: [Intel-gfx] [Beignet] [PATCH 2/2 v2] Query the driver directly for compute units and subslice
LGTM,
Reviewed-by: Zhigang Gong
Thanks.
> -Original Message-
> From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of
> jeff.mc...@intel.com
> Sent: Tuesday, March 10, 2015 7:36 AM
> To: beig...@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Subject: [Beignet] [PATCH 2/2 v2] Query the driver directly for compute units
> and subslice
>
> From: Jeff McGee
>
> Values of device max compute units and max subslice obtained directly from
> the driver should be more accurate than our own ID-based lookup values. This
> is particularly important when a single device ID may encompass more than
> one configuration. If the driver cannot provide a valid value for the given
> device,
> we fallback on the ID-based lookup value.
>
> This query requires libdrm 2.4.60. For now we will consider the use of this
> query
> to be optional and exclude it from compilation when building against older
> libdrm. Later we may want to consider requiring the query or at least warning
> more strongly when it is not supported.
>
> v2: Make feature use conditional on libdrm version (Zhigang).
>
> Signed-off-by: Jeff McGee
> ---
> CMakeLists.txt | 9 +
> src/CMakeLists.txt | 10 ++
> src/intel/intel_driver.c | 25 +
> 3 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt index 65f2c70..bb03566 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -131,6 +131,15 @@ IF(DRM_INTEL_FOUND)
>ELSE(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> MESSAGE(STATUS "Disable userptr support")
>ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> + IF(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
> +MESSAGE(STATUS "Enable EU total query support")
> +SET(DRM_INTEL_EU_TOTAL "enable")
> +MESSAGE(STATUS "Enable subslice total query support")
> +SET(DRM_INTEL_SUBSLICE_TOTAL "enable")
> ELSE(DRM_INTEL_VERSION
> + VERSION_GREATER 2.4.59)
> +MESSAGE(STATUS "Disable EU total query support")
> +MESSAGE(STATUS "Disable subslice total query support")
> + ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
> ELSE(DRM_INTEL_FOUND)
>MESSAGE(FATAL_ERROR "Looking for DRM Intel (>= 2.4.52) - not found")
> ENDIF(DRM_INTEL_FOUND)
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d4181d8..464765f
> 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -118,6 +118,16 @@ SET(CMAKE_CXX_FLAGS "-DHAS_USERPTR
> ${CMAKE_CXX_FLAGS}") SET(CMAKE_C_FLAGS "-DHAS_USERPTR
> ${CMAKE_C_FLAGS}") endif (DRM_INTEL_USERPTR)
>
> +if (DRM_INTEL_EU_TOTAL)
> +SET(CMAKE_CXX_FLAGS "-DHAS_EU_TOTAL ${CMAKE_CXX_FLAGS}")
> +SET(CMAKE_C_FLAGS "-DHAS_EU_TOTAL ${CMAKE_C_FLAGS}") endif
> +(DRM_INTEL_EU_TOTAL)
> +
> +if (DRM_INTEL_SUBSLICE_TOTAL)
> +SET(CMAKE_CXX_FLAGS "-DHAS_SUBSLICE_TOTAL ${CMAKE_CXX_FLAGS}")
> +SET(CMAKE_C_FLAGS "-DHAS_SUBSLICE_TOTAL ${CMAKE_C_FLAGS}") endif
> +(DRM_INTEL_SUBSLICE_TOTAL)
> +
> set(GIT_SHA1 "git_sha1.h")
> add_custom_target(${GIT_SHA1} ALL
>COMMAND chmod +x ${CMAKE_CURRENT_SOURCE_DIR}/git_sha1.sh
> diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c index
> d61988c..755ab6b 100644
> --- a/src/intel/intel_driver.c
> +++ b/src/intel/intel_driver.c
> @@ -757,10 +757,7 @@ static int intel_buffer_set_tiling(cl_buffer bo, static
> void intel_update_device_info(cl_device_id device) { -#ifdef HAS_USERPTR
>intel_driver_t *driver;
> - const size_t sz = 4096;
> - void *host_ptr;
>
>driver = intel_driver_new();
>assert(driver != NULL);
> @@ -769,6 +766,10 @@ intel_update_device_info(cl_device_id device)
> return;
>}
>
> +#ifdef HAS_USERPTR
> + const size_t sz = 4096;
> + void *host_ptr;
> +
>host_ptr = cl_aligned_malloc(sz, 4096);
>if (host_ptr != NULL) {
> cl_buffer bo = intel_buffer_alloc_userptr((cl_buffer_mgr)driver->bufmgr,
> @@ -781,12 +782,28 @@ intel_update_device_info(cl_device_id device)
>}
>else
> device->host_unified_memory = CL_FALSE;
> +#endif
> +
> +#ifdef HAS_EU_TOTAL
> + unsigned int eu_total;
> +
> + /* Prefer driver-queried max compute units if supported */
> + if (!drm_intel_get_eu_total(driver->fd, &eu_total))
> +device->max_compute_unit = eu_total; #endif
> +
> +#ifdef HAS_SUBSLICE_TOTAL
> + unsigned int subslice_total;
> +
> + /* Prefer driver-queried subslice count if supported */
> + if (!drm_intel_get_subslice_total(driver->f
Re: [Intel-gfx] [Beignet] [PATCH 1/2] Add driver callback for updating device info
This patchset is a must for beignet to support CHV. One comment is that we
should
put the usage of these new libdrm APIs to conditional block thus we don't break
the
build on old system.
For the other parts of the patchset:
Reviewed-by: Zhigang Gong
Thanks,
Zhigang Gong.
> -Original Message-
> From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of
> jeff.mc...@intel.com
> Sent: Tuesday, March 3, 2015 7:43 AM
> To: beig...@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Subject: [Beignet] [PATCH 1/2] Add driver callback for updating device info
>
> From: Jeff McGee
>
> We need to update some fields of the device's cl_device_id struct at runtime
> using driver-specific methods. It is best to group all such updates into a
> single
> driver callback to avoid opening/initing and deiniting/closing the device
> multiple
> times.
>
> Signed-off-by: Jeff McGee
> ---
> src/cl_device_id.c | 20 ++--
> src/cl_driver.h | 4
> src/cl_driver_defs.c | 1 +
> src/intel/intel_driver.c | 36
> 4 files changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/src/cl_device_id.c b/src/cl_device_id.c index 4e01c9f..fefcef3
> 100644
> --- a/src/cl_device_id.c
> +++ b/src/cl_device_id.c
> @@ -506,24 +506,8 @@ skl_gt4_break:
> ret->profile_sz = strlen(ret->profile) + 1;
>}
>
> -#ifdef HAS_USERPTR
> - cl_driver dummy = cl_driver_new(NULL);
> - cl_buffer_mgr bufmgr = cl_driver_get_bufmgr(dummy);
> -
> - const size_t sz = 4096;
> - void* host_ptr = cl_aligned_malloc(sz, 4096);;
> - if (host_ptr != NULL) {
> -cl_buffer bo = cl_buffer_alloc_userptr(bufmgr, "CL memory object",
> host_ptr, sz, 0);
> -if (bo == NULL)
> - ret->host_unified_memory = CL_FALSE;
> -else
> - cl_buffer_unreference(bo);
> -cl_free(host_ptr);
> - }
> - else
> -ret->host_unified_memory = CL_FALSE;
> - cl_driver_delete(dummy);
> -#endif
> + /* Apply any driver-dependent updates to the device info */
> + cl_driver_update_device_info(ret);
>
>struct sysinfo info;
>if (sysinfo(&info) == 0) {
> diff --git a/src/cl_driver.h b/src/cl_driver.h index 16f8bba..3f54a27 100644
> --- a/src/cl_driver.h
> +++ b/src/cl_driver.h
> @@ -376,6 +376,10 @@ extern cl_buffer_get_tiling_align_cb
> *cl_buffer_get_tiling_align; typedef int (cl_driver_get_device_id_cb)(void);
> extern cl_driver_get_device_id_cb *cl_driver_get_device_id;
>
> +/* Update the device info */
> +typedef void (cl_driver_update_device_info_cb)(cl_device_id device);
> +extern cl_driver_update_device_info_cb *cl_driver_update_device_info;
> +
>
> /***
> ***
> * cl_khr_gl_sharing.
>
>
> **/
> diff --git a/src/cl_driver_defs.c b/src/cl_driver_defs.c index
> 2b68539..9a47210
> 100644
> --- a/src/cl_driver_defs.c
> +++ b/src/cl_driver_defs.c
> @@ -26,6 +26,7 @@ LOCAL cl_driver_delete_cb *cl_driver_delete = NULL;
> LOCAL cl_driver_get_bufmgr_cb *cl_driver_get_bufmgr = NULL; LOCAL
> cl_driver_get_ver_cb *cl_driver_get_ver = NULL; LOCAL
> cl_driver_get_device_id_cb *cl_driver_get_device_id = NULL;
> +LOCAL cl_driver_update_device_info_cb *cl_driver_update_device_info =
> +NULL;
>
> /* Buffer */
> LOCAL cl_buffer_alloc_cb *cl_buffer_alloc = NULL; diff --git
> a/src/intel/intel_driver.c b/src/intel/intel_driver.c index ff0cf27..d61988c
> 100644
> --- a/src/intel/intel_driver.c
> +++ b/src/intel/intel_driver.c
> @@ -754,6 +754,41 @@ static int intel_buffer_set_tiling(cl_buffer bo,
>return ret;
> }
>
> +static void
> +intel_update_device_info(cl_device_id device) { #ifdef HAS_USERPTR
> + intel_driver_t *driver;
> + const size_t sz = 4096;
> + void *host_ptr;
> +
> + driver = intel_driver_new();
> + assert(driver != NULL);
> + if (intel_driver_open(driver, NULL) != CL_SUCCESS) {
> +intel_driver_delete(driver);
> +return;
> + }
> +
> + host_ptr = cl_aligned_malloc(sz, 4096); if (host_ptr != NULL) {
> +cl_buffer bo = intel_buffer_alloc_userptr((cl_buffer_mgr)driver->bufmgr,
> + "CL memory object", host_ptr, sz, 0);
> +if (bo == NULL)
> + device->host_unified_memory = CL_FALSE;
> +else
> + drm_intel_bo_unreference((drm_intel_bo*)bo);
> +cl_free(host_ptr);
> + }
> + else
> +device->host_unified_memory = CL_FALSE;
> +
> + intel_driver_context_destroy(driver);
> + inte
Re: [Intel-gfx] [Beignet] [PATCH] drm/i915: Export total subslice and EU counts
> -Original Message-
> From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of
> Jeff McGee
> Sent: Saturday, March 7, 2015 2:44 AM
> To: Zhigang Gong
> Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org;
> beig...@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] drm/i915: Export total subslice and EU counts
>
> On Thu, Mar 05, 2015 at 12:35:55PM +0800, Zhigang Gong wrote:
> > There is one minor conflict when apply the KMD patch to latest
> > drm-intel-nightly branch. It should be easy to fix.
> >
> > Another issue is that IMO, we should bump libdrm's version number when
> > increase these new APIs. Then in Beignet, we can check the libdrm
> > version at build time and determine whether we will use these new
> > interfaces. Thus, we can avoid breaking beignet on those systems which
> > have previous libdrm/kernel installed.
> >
> Right. I can append a libdrm patch to bump the version. And then I suppose I
> will follow the process to make a new release. Not sure right now how that
> works. First time going through it.
>
> Also, how should we test for the libdrm version and conditionally use the API?
We can check the libdrm version at configuration time and define a macro to
indicate whether we can use these new APIs in beignet.
> Is there a previous example of this in Beignet that I could follow?
Yes, one example is userptr. You can check the usage of DRM_INTEL_USERPTR and
HAS_USERPTR
In beignet.
Thanks,
Zhigang Gong.
>
> Jeff
>
> > The other parts of the whole patchset, including patches for
> > KMD/libdrm/Intel gpu tools and Beignet, all look good to me.
> >
> > And I just tested it on BDW and SKL platforms, it works fine.
> >
> > Thanks,
> > Zhigang Gong.
> >
> > On Mon, Mar 02, 2015 at 03:37:32PM -0800, jeff.mc...@intel.com wrote:
> > > From: Jeff McGee
> > >
> > > Setup new I915_GETPARAM ioctl entries for subslice total and EU
> > > total. Userspace drivers need these values when constructing GPGPU
> > > commands. This kernel query method is intended to replace the PCI
> > > ID-based tables that userspace drivers currently maintain.
> > > The kernel driver can employ fuse register reads as needed to ensure
> > > the most accurate determination of GT config attributes.
> > > This first became important with Cherryview in which the config
> > > could differ between devices with the same PCI ID.
> > >
> > > The kernel detection of these values is device-specific and not
> > > included in this patch. Because zero is not a valid value for any of
> > > these parameters, a value of zero is interpreted as unknown for the
> > > device. Userspace drivers should continue to maintain ID-based
> > > tables for older devices not supported by the new query method.
> > >
> > > For: VIZ-4636
> > > Signed-off-by: Jeff McGee
> > > ---
> > > drivers/gpu/drm/i915/i915_dma.c | 10 ++
> > > include/uapi/drm/i915_drm.h | 2 ++
> > > 2 files changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > > b/drivers/gpu/drm/i915/i915_dma.c index 053e178..9350ea2 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -150,6 +150,16 @@ static int i915_getparam(struct drm_device *dev,
> void *data,
> > > case I915_PARAM_MMAP_VERSION:
> > > value = 1;
> > > break;
> > > + case I915_PARAM_SUBSLICE_TOTAL:
> > > + value = INTEL_INFO(dev)->subslice_total;
> > > + if (!value)
> > > + return -ENODEV;
> > > + break;
> > > + case I915_PARAM_EU_TOTAL:
> > > + value = INTEL_INFO(dev)->eu_total;
> > > + if (!value)
> > > + return -ENODEV;
> > > + break;
> > > default:
> > > DRM_DEBUG("Unknown parameter %d\n", param->param);
> > > return -EINVAL;
> > > diff --git a/include/uapi/drm/i915_drm.h
> > > b/include/uapi/drm/i915_drm.h index 6eed16b..8672efc 100644
> > > --- a/include/uapi/drm/i915_drm.h
> > > +++ b/include/uapi/drm/i915_drm.h
> > > @@ -347,6 +347,8 @@ typedef struct drm_i915_irq_wait { #define
> > > I915_PARAM_HAS_COHERENT_PHYS_GTT 29
> > > #define I915_PARAM_MMAP_VERSION 30
> > > #define I915_PARAM_HAS_BSD2 31
> > > +#define I915_PARAM_
Re: [Intel-gfx] [Beignet] [PATCH] drm/i915: Export total subslice and EU counts
There is one minor conflict when apply the KMD patch to latest
drm-intel-nightly branch. It should be easy to fix.
Another issue is that IMO, we should bump libdrm's version number
when increase these new APIs. Then in Beignet, we can check the
libdrm version at build time and determine whether we will use
these new interfaces. Thus, we can avoid breaking beignet on
those systems which have previous libdrm/kernel installed.
The other parts of the whole patchset,
including patches for KMD/libdrm/Intel gpu tools and Beignet,
all look good to me.
And I just tested it on BDW and SKL platforms, it works fine.
Thanks,
Zhigang Gong.
On Mon, Mar 02, 2015 at 03:37:32PM -0800, jeff.mc...@intel.com wrote:
> From: Jeff McGee
>
> Setup new I915_GETPARAM ioctl entries for subslice total and
> EU total. Userspace drivers need these values when constructing
> GPGPU commands. This kernel query method is intended to replace
> the PCI ID-based tables that userspace drivers currently maintain.
> The kernel driver can employ fuse register reads as needed to
> ensure the most accurate determination of GT config attributes.
> This first became important with Cherryview in which the config
> could differ between devices with the same PCI ID.
>
> The kernel detection of these values is device-specific and not
> included in this patch. Because zero is not a valid value for any of
> these parameters, a value of zero is interpreted as unknown for the
> device. Userspace drivers should continue to maintain ID-based tables
> for older devices not supported by the new query method.
>
> For: VIZ-4636
> Signed-off-by: Jeff McGee
> ---
> drivers/gpu/drm/i915/i915_dma.c | 10 ++
> include/uapi/drm/i915_drm.h | 2 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 053e178..9350ea2 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -150,6 +150,16 @@ static int i915_getparam(struct drm_device *dev, void
> *data,
> case I915_PARAM_MMAP_VERSION:
> value = 1;
> break;
> + case I915_PARAM_SUBSLICE_TOTAL:
> + value = INTEL_INFO(dev)->subslice_total;
> + if (!value)
> + return -ENODEV;
> + break;
> + case I915_PARAM_EU_TOTAL:
> + value = INTEL_INFO(dev)->eu_total;
> + if (!value)
> + return -ENODEV;
> + break;
> default:
> DRM_DEBUG("Unknown parameter %d\n", param->param);
> return -EINVAL;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 6eed16b..8672efc 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -347,6 +347,8 @@ typedef struct drm_i915_irq_wait {
> #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
> #define I915_PARAM_MMAP_VERSION 30
> #define I915_PARAM_HAS_BSD2 31
> +#define I915_PARAM_SUBSLICE_TOTAL 32
> +#define I915_PARAM_EU_TOTAL 33
>
> typedef struct drm_i915_getparam {
> int param;
> --
> 2.3.0
>
> ___
> Beignet mailing list
> beig...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/hsw: enable atomic in L3 for some steppings.
On Mon, Jan 05, 2015 at 07:27:26PM +0200, Francisco Jerez wrote: > Zhigang Gong writes: > > > On Mon, Jan 05, 2015 at 05:03:16AM +0200, Francisco Jerez wrote: > >> Zhigang Gong writes: > >> > >> > According to bspec, ROW_CHICKEN3's bit 6 which is to disable > >> > move of cacheable global atomics to L3 is needed for GT3 D > >> > stepping. > >> > > >> > I enabled it and tested it with HSW GT2 D stepping and GT3 E stepping. > >> > The atomics works fine in beignet. And it could get more than 10x > >> > performance > >> > improvement with some workload, for an example, the "splat" kernel in > >> > darktable, > >> > without this patch, it consumes 50 seconds in one large raw picture > >> > processing. > >> > But with this patch, the same process only takes less than 1 second. > >> > > >> > >> I tried this already (on HSW GT2 D as well) and I don't think it's > >> enough to get L3 atomics working reliably. Even though they did seem to > >> work OK at first glance I observed some corruption issues (e.g. atomic > >> writes not landing in system memory) when doing atomic writes to > >> contiguous (as in within the same cache-line) locations in memory. The > >> "unused" ARB_shader_image_load_store test [1] I sent to the Piglit > >> mailing list some time ago exposes this IIRC, and probably a couple of > >> other tests too. > > Ok, I will find that case and have a try on my systems. I just tested all > > the atomic related OpenCL conformance test cases without any issues. I just found the patchset hasn't been accepted by piglit. I took a look at the source code. And realize that that shader will not work correctly if mesa doesn't allocate some DC in L3 space. Don't know whether you already allocated some DC when you did that test. > > > >> > >> Also this change is going to cause an instant lock-up anytime Mesa uses > >> atomics because Mesa doesn't change the default L3 way allocation for > >> the DC, which turns out to be 0 on HSW. > > > > This is another issue, IMHO, if the application wants to use atomics, > > it's better to allocate some L3 space for DC. Otherwise, it could > > never leverage the "atomics in L3 feature". Based on my test, > > the performance impact as huge as more than 10x for some workloads. > > > Sure, but this change alone will cause a regression (an irrecoverable > system hang) with current releases of Mesa. > > I agree that Mesa should be fixed eventually to assign L3 space to the > DC for some workloads, but it doesn't seem like we have a satisfactory > API to do that right now -- The current mechanism used by Beignet > (poking the L3 control registers from the batch buffer) is slightly > concerning from a security point of view because it allows an arbitrary > userspace application to cause misrendering or impact the performance of I agree with you that the L3 configuration is very dangerous. It's better to do more checking in the kernel space and make sure the user space application will not crash the system via L3 configuration. And, if the kernel provide new API we will use it in beignet ASAP in beignet. > other clients (since the L3 config registers are not being saved and > restored as part of the context) and even crash the whole system. It I found the L3 config registers are part of the IVB context, but not part of HSW's. So if two user applications want to use different L3 config, the user application need to do the config registers' store/restore manually. Is there a way to do this type of thing gracefully in KMD? The new API may need to consider the safely context switching regards to L3 config change. > also doesn't look like it could be made to work with the hardware > command checker. As to the hardware command checker: I just found with current nightly kernel, if we boot the kernel with "i915.enable_ppgtt=2", then we can configure the L3 related registers in user space. But the command checker still don't allow user space to chagne L3 config by default. Thanks, Zhigang Gong. > > > Thanks, > > Zhigang Gong. > > > >> > >> [1] http://lists.freedesktop.org/archives/piglit/2014-December/013571.html > >> > >> > Signed-off-by: Zhigang Gong > >> > --- > >> > drivers/gpu/drm/i915/intel_pm.c | 10 ++ > >> > 1 file changed, 6 insertions(+), 4 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_pm.c >
Re: [Intel-gfx] [PATCH] drm/i915/hsw: enable atomic in L3 for some steppings.
On Mon, Jan 05, 2015 at 05:03:16AM +0200, Francisco Jerez wrote:
> Zhigang Gong writes:
>
> > According to bspec, ROW_CHICKEN3's bit 6 which is to disable
> > move of cacheable global atomics to L3 is needed for GT3 D
> > stepping.
> >
> > I enabled it and tested it with HSW GT2 D stepping and GT3 E stepping.
> > The atomics works fine in beignet. And it could get more than 10x
> > performance
> > improvement with some workload, for an example, the "splat" kernel in
> > darktable,
> > without this patch, it consumes 50 seconds in one large raw picture
> > processing.
> > But with this patch, the same process only takes less than 1 second.
> >
>
> I tried this already (on HSW GT2 D as well) and I don't think it's
> enough to get L3 atomics working reliably. Even though they did seem to
> work OK at first glance I observed some corruption issues (e.g. atomic
> writes not landing in system memory) when doing atomic writes to
> contiguous (as in within the same cache-line) locations in memory. The
> "unused" ARB_shader_image_load_store test [1] I sent to the Piglit
> mailing list some time ago exposes this IIRC, and probably a couple of
> other tests too.
Ok, I will find that case and have a try on my systems. I just tested all
the atomic related OpenCL conformance test cases without any issues.
>
> Also this change is going to cause an instant lock-up anytime Mesa uses
> atomics because Mesa doesn't change the default L3 way allocation for
> the DC, which turns out to be 0 on HSW.
This is another issue, IMHO, if the application wants to use atomics,
it's better to allocate some L3 space for DC. Otherwise, it could
never leverage the "atomics in L3 feature". Based on my test,
the performance impact as huge as more than 10x for some workloads.
Thanks,
Zhigang Gong.
>
> [1] http://lists.freedesktop.org/archives/piglit/2014-December/013571.html
>
> > Signed-off-by: Zhigang Gong
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 7d99a9c..8a27802 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5938,10 +5938,12 @@ static void haswell_init_clock_gating(struct
> > drm_device *dev)
> >
> > ilk_init_lp_watermarks(dev);
> >
> > - /* L3 caching of data atomics doesn't work -- disable it. */
> > - I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
> > - I915_WRITE(HSW_ROW_CHICKEN3,
> > -
> > _MASKED_BIT_ENABLE(HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE));
> > + if (IS_HSW_GT3(dev) && dev->pdev->revision <= 6) {
> > + /* L3 caching of data atomics doesn't work -- disable it. */
> > + I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
> > + I915_WRITE(HSW_ROW_CHICKEN3,
> > +
> > _MASKED_BIT_ENABLE(HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE));
> > + }
> >
> > /* This is required by WaCatErrorRejectionIssue:hsw */
> > I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
> > --
> > 1.8.3.2
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/hsw: enable atomic in L3 for some steppings.
According to bspec, ROW_CHICKEN3's bit 6 which is to disable
move of cacheable global atomics to L3 is needed for GT3 D
stepping.
I enabled it and tested it with HSW GT2 D stepping and GT3 E stepping.
The atomics works fine in beignet. And it could get more than 10x performance
improvement with some workload, for an example, the "splat" kernel in darktable,
without this patch, it consumes 50 seconds in one large raw picture processing.
But with this patch, the same process only takes less than 1 second.
Signed-off-by: Zhigang Gong
---
drivers/gpu/drm/i915/intel_pm.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7d99a9c..8a27802 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5938,10 +5938,12 @@ static void haswell_init_clock_gating(struct drm_device
*dev)
ilk_init_lp_watermarks(dev);
- /* L3 caching of data atomics doesn't work -- disable it. */
- I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
- I915_WRITE(HSW_ROW_CHICKEN3,
-
_MASKED_BIT_ENABLE(HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE));
+ if (IS_HSW_GT3(dev) && dev->pdev->revision <= 6) {
+ /* L3 caching of data atomics doesn't work -- disable it. */
+ I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
+ I915_WRITE(HSW_ROW_CHICKEN3,
+
_MASKED_BIT_ENABLE(HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE));
+ }
/* This is required by WaCatErrorRejectionIssue:hsw */
I915_WRITE(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Beignet crashes on vanilla 3.17.1 with IVB hardware
Vasily, Could you try ImageMagick(convert) again with latest git master beignet(git-e46764f). It should work now. Thanks, Zhigang Gong. On Fri, Oct 24, 2014 at 04:36:49PM +0300, Vasily Khoruzhick wrote: > Hi Zhigang, > > On Fri, Oct 24, 2014 at 12:13 PM, Zhigang Gong > wrote: > > Hi, > > > > Luxmark (both 2.0/2.1) works fine on my IVB machine. The back trace > > you provided below doesn't indicate it's a beignet related problem. > > It hadn't enter beignet domain and just crashed in luxmark internal. > > I'm testing with Luxmark-1.3.1. Luxmark 2.0 works fine. > > > On Fri, Oct 24, 2014 at 12:04:29PM +0300, Vasily Khoruzhick wrote: > >> Hi Zhigang, > >> > >> Luxmark crashes with following backtrace: > >> > >> Program received signal SIGSEGV, Segmentation fault. > >> 0x004cd8b0 in slg::PathOCLRenderEngine::StopLockLess() () > >> (gdb) bt > >> #0 0x004cd8b0 in slg::PathOCLRenderEngine::StopLockLess() () > >> #1 0x00482236 in slg::RenderEngine::Stop() () > >> #2 0x0047be9b in slg::RenderSession::~RenderSession() () > >> #3 0x00468a77 in LuxMarkApp::Stop() () > >> #4 0x00468b46 in LuxMarkApp::InitRendering(LuxMarkAppMode, > >> char const*) () > > > > After a quick analysis, I confirm that the second case is indeed a beignet > > bug. Beignet lacks of some llvm intrinsics support such as > > llvm.uadd.with.overflow.i32(). > > > > Will fix it next week. > > Ok, thank you! > > > Thanks, > > Zhigang Gong. > > Regards > Vasily > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Beignet crashes on vanilla 3.17.1 with IVB hardware
Hi, Luxmark (both 2.0/2.1) works fine on my IVB machine. The back trace you provided below doesn't indicate it's a beignet related problem. It hadn't enter beignet domain and just crashed in luxmark internal. On Fri, Oct 24, 2014 at 12:04:29PM +0300, Vasily Khoruzhick wrote: > Hi Zhigang, > > Luxmark crashes with following backtrace: > > Program received signal SIGSEGV, Segmentation fault. > 0x004cd8b0 in slg::PathOCLRenderEngine::StopLockLess() () > (gdb) bt > #0 0x004cd8b0 in slg::PathOCLRenderEngine::StopLockLess() () > #1 0x00482236 in slg::RenderEngine::Stop() () > #2 0x0047be9b in slg::RenderSession::~RenderSession() () > #3 0x00468a77 in LuxMarkApp::Stop() () > #4 0x00468b46 in LuxMarkApp::InitRendering(LuxMarkAppMode, > char const*) () After a quick analysis, I confirm that the second case is indeed a beignet bug. Beignet lacks of some llvm intrinsics support such as llvm.uadd.with.overflow.i32(). Will fix it next week. Thanks, Zhigang Gong. > > And convert from imagemagick crashes in libgbe.so: > > Program received signal SIGSEGV, Segmentation fault. > 0x7fffeb0df18c in gbe::buildRegInfo(gbe::ir::BasicBlock&, > gbe::vector&) () from /usr/lib/beignet//libgbe.so > (gdb) bt > #0 0x7fffeb0df18c in gbe::buildRegInfo(gbe::ir::BasicBlock&, > gbe::vector&) () from /usr/lib/beignet//libgbe.so > #1 0x7fffeb0e048d in > gbe::GenWriter::removeLOADIs(gbe::ir::Liveness const&, > gbe::ir::Function&) () from /usr/lib/beignet//libgbe.so > #2 0x7fffeb0fb823 in > gbe::GenWriter::emitFunction(llvm::Function&) () from > /usr/lib/beignet//libgbe.so > #3 0x7fffeb100b63 in ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Beignet crashes on vanilla 3.17.1 with IVB hardware
I just checked again, and found both 3.17 and 3.17.1 should work fine on IVB with beignet. I just tested beignet on IVB with kernel 3.17.1, all unit tests passed successfully. For IVB user, no need to wait for 3.18. Don't know which application you were testing on your IVB machine. If it could be reproduced easily, please open a bug on fd.o. Thanks, Zhigang Gong. On Fri, Oct 24, 2014 at 10:27:23AM +0800, Zhigang Gong wrote: > Hi, > > For IVB, I just checked the 3.18-rc1, it has the following patch: > commit c9224faa59c3071ecfa2d4b24592f4eb61e57069 > Author: Brad Volkin > Date: Tue Jun 17 14:10:34 2014 -0700 > > drm/i915: Add some L3 registers to the parser whitelist > > Beignet needs these in order to program the L3 cache config for > OpenCL workloads, particularly when using SLM. > > Signed-off-by: Brad Volkin > Signed-off-by: Daniel Vetter > > So, beignet should work fine with 3.18 on IVB/BYT. > > But for the HSW，I'm not quite sure when we could get a workable vanilla > kernel. Something I found at the intel-gfx mail list as below and it doesn't > sound good. > > http://lists.freedesktop.org/archives/intel-gfx/2014-May/044694.html > http://lists.freedesktop.org/archives/intel-gfx/2014-May/045088.html > > CC to intel-gfx mail list. Hope we can get an official anwser here. > > Thanks, > Zhigang Gong. > > On Thu, Oct 23, 2014 at 03:39:35PM +0300, Vasily Khoruzhick wrote: > > Hi, > > > > As you maybe know, any application which uses beignet OpenCL > > implementation crashes on Ivy Bridge hardware when using vanilla > > 3.17.1 kernel. > > I guess it's due to batchbuffer security and patch to disable > > batchbuffer security is required, but guys, it fails since 3.16, and > > 3.17 was released quite a while ago. > > > > Could you cooperate with i915 driver devs to make Beignet working on > > vanilla kernel without extra patches? > > > > Thanks! > > > > Regards, > > Vasily > > ___ > > Beignet mailing list > > beig...@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/beignet > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Beignet crashes on vanilla 3.17.1 with IVB hardware
Hi, For IVB, I just checked the 3.18-rc1, it has the following patch: commit c9224faa59c3071ecfa2d4b24592f4eb61e57069 Author: Brad Volkin Date: Tue Jun 17 14:10:34 2014 -0700 drm/i915: Add some L3 registers to the parser whitelist Beignet needs these in order to program the L3 cache config for OpenCL workloads, particularly when using SLM. Signed-off-by: Brad Volkin Signed-off-by: Daniel Vetter So, beignet should work fine with 3.18 on IVB/BYT. But for the HSW，I'm not quite sure when we could get a workable vanilla kernel. Something I found at the intel-gfx mail list as below and it doesn't sound good. http://lists.freedesktop.org/archives/intel-gfx/2014-May/044694.html http://lists.freedesktop.org/archives/intel-gfx/2014-May/045088.html CC to intel-gfx mail list. Hope we can get an official anwser here. Thanks, Zhigang Gong. On Thu, Oct 23, 2014 at 03:39:35PM +0300, Vasily Khoruzhick wrote: > Hi, > > As you maybe know, any application which uses beignet OpenCL > implementation crashes on Ivy Bridge hardware when using vanilla > 3.17.1 kernel. > I guess it's due to batchbuffer security and patch to disable > batchbuffer security is required, but guys, it fails since 3.16, and > 3.17 was released quite a while ago. > > Could you cooperate with i915 driver devs to make Beignet working on > vanilla kernel without extra patches? > > Thanks! > > Regards, > Vasily > ___ > Beignet mailing list > beig...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/beignet ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] glamor and xvmc
Dave Airlie just implemented some Xv support as below, although not fully support all functions. But currently, we haven't enable it in Intel's driver side. commit bad96d415525b12add517b09a26b52c2c36979f7 Author: Dave Airlie Date: Mon Sep 23 06:42:24 2013 +0100 glamor: add initial Xv support This does YV12 and I420 for now, not sure if we can do packed without a GL extension. -Original Message- From: intel-gfx-bounces+zhigang.gong=linux.intel@lists.freedesktop.org [mailto:intel-gfx-bounces+zhigang.gong=linux.intel@lists.freedesktop.org ] On Behalf Of Zou, Nanhai Sent: Wednesday, October 23, 2013 10:39 AM To: Chris Wilson; Grant Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] glamor and xvmc I don't think glamor support xmvc now. However port those xmvc code to glamor is doable. Thanks Zou Nanhai -Original Message- From: intel-gfx-bounces+nanhai.zou=intel@lists.freedesktop.org [mailto:intel-gfx-bounces+nanhai.zou=intel@lists.freedesktop.org] On Behalf Of Chris Wilson Sent: Tuesday, October 22, 2013 4:09 PM To: Grant Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] glamor and xvmc On Tue, Oct 22, 2013 at 01:02:22AM -0700, Grant wrote: > > With the exception of not having Xv (or XvMC) support enabled for > > glamor in -intel, you can, at your own risk, enable glamor during > > configure > > Are you saying that glamor depends on xvmc? -intel needs to explicitly enable Xv support in its glamor backend, which it does not yet do. I'm not sure if glamor could expose the hardware motion compensation acceleration though. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] FW: Release Notes for Beignet Version 0.1
> -Original Message- > From: > intel-gfx-bounces+zhigang.gong=linux.intel@lists.freedesktop.org > [mailto:intel-gfx-bounces+zhigang.gong=linux.intel.com@lists.freedesktop. > org] On Behalf Of Zou, Nanhai > Sent: Tuesday, April 16, 2013 2:47 PM > To: Dave Airlie > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] FW: Release Notes for Beignet Version 0.1 > > Hi, > > > Again not really important, we use LLVM and TGSI backends in the > radeon drivers now. The gallium drivers currently use GPU specific LLVM > IRs produced from specific LLVM backends, but that isn't strictly necessary > just leads to better optimising opportunities. > > Because you now have a whole bunch of code that is useless to anyone > else. Distro want to ship this sort of thing, we don't want 5 Mesa like > implementations for OpenCL, we want one we can actually distribute and > manage, and >maybe in 5 years support. > > The point is at the current stage, we don't see the necessary of > introduce all those complexity and dependencies. > Considering we can share very few code with other part of > Mesa(expect some GPU related define in header files). > > We want to keep this project tiny and self-contained at this point, > focus on make OpenCL really fast and useful on IVB+ GPU. > > We can merge or hook the backend into gallium when there are > other mature gallium based OpenCL implementations. > That should not be a lot of task, consider the common code are very > few. > > > > Some other questions: > > a) have you got an ivybridge LLVM backend? are you going to upstream > this, I heard it isn't even written in LLVM machine description format. > > No, we convert llvm IR to Gen IR then to IVB assembly. > There are many choices when implement an execute model of IVB. E.g > 8 way or 16 way, SOA mode or AOS mode. > We choose 16 way AOS mode in the driver. > I don't know if it make sense to upstream only this particular > combination. > > > > b) have you looked at pocl, libclc etc? Maybe you guys want to run on > CPU as well at GPU at some point, in which case maybe again looking > around before jumping into implementing stuff might help. > > We do have looked at libclc. > Again, I don't see the necessary to introduce the complexity at this > point > Almost all the functions in libclc maps to 1 instructions on IVB. We do > not see the necessary of introducing a library to wrap those. > > > c) does this use the open source ICD at least? > No, we will check that. [Gong, Zhigang] As I know, Simon implemented ICD for Beignet and it may need some time to rebase to the latest beignet code base. If you are interested, this is the link http://psi5.com/~geier/beignet.git. > > > Dave. > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [ANNOUNCE] glamor 0.5.0
On Tue, Aug 21, 2012 at 12:51 PM, Rémi Cardona wrote: > Hi Zhigang, > > Le vendredi 10 août 2012 à 18:37 +0800, Zhigang Gong a écrit : >> tar ball is at: >> >> http://cgit.freedesktop.org/xorg/driver/glamor/snapshot/glamor-0.5.tar.gz > > This is a git snapshot, not a tarball made using automake's "make dist" > like all X.org modules use. I forgot to make and upload the tarballs. Thanks for your reminder. > > Is one planned? sure, here you are. git tag: glamor-egl-0.5.0 http://xorg.freedesktop.org/archive/individual/driver/glamor-egl-0.5.0.tar.bz2 MD5: 7dc2b4e0d8d286531c433893945a3837 glamor-egl-0.5.0.tar.bz2 SHA1: 6495d1a2054e2a8e0f7e01bc7e3345cd50d21972 glamor-egl-0.5.0.tar.bz2 SHA256: 5dc8679ccb3e42bf431b6316c7907b9df2db89745d523e04721f34aee6c84991 glamor-egl-0.5.0.tar.bz2 http://xorg.freedesktop.org/archive/individual/driver/glamor-egl-0.5.0.tar.gz MD5: 8f8f58cbf661a6eb70785864ebf5153f glamor-egl-0.5.0.tar.gz SHA1: 08366dd52b5f5812847f20a1252a86100766f8ab glamor-egl-0.5.0.tar.gz SHA256: e7ed6ead72a44bcd68d837eb5bf133f760be1e5fb42907cf1bb06ba5f4d6dc18 glamor-egl-0.5.0.tar.gz > > Cheers, > > Rémi > > ___ > x...@lists.x.org: X.Org support > Archives: http://lists.freedesktop.org/archives/xorg > Info: http://lists.x.org/mailman/listinfo/xorg > Your subscription address: zhigang.g...@gmail.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Netiquette: No HTML messages please. (was: [ANNOUNCE] glamor 0.5.0)
On Sat, Aug 11, 2012 at 10:55:08AM +0200, Paul Menzel wrote: > Dear Zhigang, > > > thank you for the announcement message and congratulations on the > release. But please do not send HTML messages to lists [1]! Thanks for pointing this out. Actually, I was not aware of that I sent any non plain-text messages. The email client did some auto formating for me :(. I will check the message more carefully next time. Thanks, Zhigang > > > Thanks, > > Paul > > > [1] http://en.opensuse.org/openSUSE:Mailing_list_netiquette ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [ANNOUNCE] glamor 0.5.0
On Fri, Aug 10, 2012 at 08:58:33AM -0700, Jeremy Huddleston Sequoia wrote: > > On Aug 10, 2012, at 03:37, Zhigang Gong wrote: > > > to try a full functional xserver with glamor, it’s recommended to use the > > following xserver version: > > > > commit a615b90cab7569fae9d123e4da1d3373c871d84b > > > > Author: Keith Packard > > > > Date: Wed Mar 14 11:32:36 2012 -0700 > > > > > > > >Bump version number to 1.12.99.0 > > > > > > > >Now that 1.12 has branched, reset the version on master to a > > > >development number. > > > > > Why is such an old server version recommended? Surely tip of > server-1.12-branch is superior to this branch point+1 on master? And I'd > really expect tip of master to be a better candidate than that given the > development nature of glamor. Can you please clarify? This version is what I'm used to do the last round testing, so I recommended it in the announcement. Actually, glamor 0.5.0 is also compatible with the tip of server-1.12-branch. I just not do fully testing with it but it should work fine. As to the master branch, glamor 0.5.0 is not compatible with it since the following commit: a1d41e...: Move extension initialisation prototypes into extinit. The last compatible version is at commit b86aa74... As to the reason why glamor can't work with tip ot master, please refer: http://lists.x.org/archives/xorg-devel/2012-July/032597.html. BTW, to fix that issue is the first to do item for next version glamor. Thanks, Zhigang > > Thanks, > Jeremy > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [ANNOUNCE] glamor 0.5.0
After three moths development, Im pleased to release glamor version 0.5.0. The major improvements are as below: 1. Support tiling large pixmap to multiple small textures. 2. Enable gradient shader. 3. Optimize glyphs rendering performance 4. Implement first shader to generate trapezoids. Cairo-demos spinner on a large picture(12000x12000) get about 10x improvement, and gradient also get about 2x improvement. Aa10text and rgb10text also get about 2x improvement. Thanks for Junyan and Michels contribution. Glamor has a restriction with latest xserver. The main issue is that glamor rely on the module loading sequence which is not guaranteed by current xserver. We will fix this issue in next version. If you want to try a full functional xserver with glamor, its recommended to use the following xserver version: commit a615b90cab7569fae9d123e4da1d3373c871d84b Author: Keith Packard Date: Wed Mar 14 11:32:36 2012 -0700 Bump version number to 1.12.99.0 Now that 1.12 has branched, reset the version on master to a development number. Signed-off-by: Keith Packard kei...@keithp.com We have the following work items for next release: 1. Fix the coexisting problem with glx for latest xserver. Dont rely on the module loading sequence. 2. Continue performance tuning: a) Implement delay flushing mechanism to avoid tiny drawing operation for each DrawElements/DrawArrays call. b) Implement atlas for small pixmap. c) Optimize trapezoid shader to reduce the overhead for those non-edge pixels. d) Optimize trapezoid/gradient shader to merge the mask/source creation and compositing into one shader. e) Optimize glamor itself overhead. 3. Xv support. The following changes since commit d5cdad0497ae5f6cd936a74f68169c0910ea1e68: Release 0.4. (2012-04-28 17:02:35 +0800) are available in the git repository at: git://anongit.freedesktop.org/xorg/driver/glamor tag v0.5 tar ball is at: http://cgit.freedesktop.org/xorg/driver/glamor/snapshot/glamor-0.5.tar.gz Please refer the following wiki page for how to try glamor with Intel driver. http://www.freedesktop.org/wiki/Software/Glamor for you to fetch changes up to 9b8a791290af0add84269efdb315f9f58798f6d2: Bump to version 0.5. (2012-08-10 13:46:42 +0800) Junyan He (14): Extract the gradient related code out. Fix a bugy macro definition. Fix the problem of set the same stop several times. Fix the problem of vertical and horizontal case error in linear gradient. Fix the problem of x_source and y_source causing radial error Add macro of vertex setting for triangle stripe Modilfy the composite logic to two phases Add the trapezoid direct render logic Use the direct render path for A1 Fix a bug for trapezoid clip Change the trapezoid render to use VBO. Just use the shader to generate trapezoid if PolyMode == Imprecise Fix the problem of VBO leak. Fallback to pixman when trapezoid mask is big. Michel Dänzer (4): Fix translation of clip region for composite fallback. Stream vertex data to VBOs. Print space between name of missing EGL extension and 'required'. Prefer KHR_surfaceless_context EGL extension over KHR_surfaceless_opengl/gles2. RobinHe (2): Create the file glamor_triangles.c Use shader to generate the temp trapezoid mask Zhigang Gong (43): We should not call gradient finalization code if we disable it. Added strict warning flags to CFLAGS. Remove the texture cache code. glamor_set_destination_pixmap_priv_nc: set drawable's width x height. glamor_largepixmap: first commit for large pixmap. largepixmap: Implement infrastructure for large pixmap. largepixmap: Enable glamor_composite. glamor_putimage: Correct the wrong stride value. glamor_getimage: should call miGetimage if failed to get sub-image. largepixmap: Add transform/repeat/reflect/pad support. largepixmap: Support self composite for large pixmap. largepixmap: Fix the selfcopy issue. Enable large pixmap by default. trapezoid: Fallback to sw-rasterize for largepixmap. copyarea: Cleanup the error handling logic. glamor_glyphs: Before get upload to cache flush is needed. glamor_emit_composite_vert: Optimize to don't do two times vert coping. gles2_largepixmap: force clip for a non-large pixmap. glamor_fbo: fix a memory leak for large pixmap. glamor_create_pixmap: Allocate glyphs pixmap in memory. glamor_render: Don't fallback when rendering glyphs with OpOver. glamor_glyphs: Detect fake or real glyphs overlap. glamor_composite_glyph:
Re: [Intel-gfx] [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.
On Fri, Jul 27, 2012 at 10:10:32AM +0100, Chris Wilson wrote:
> On Fri, 27 Jul 2012 17:06:21 +0800, zhigang.g...@linux.intel.com wrote:
> > - if (attachment == DRI2BufferFrontLeft)
> > + if (attachment == DRI2BufferFrontLeft && is_glamor_pixmap)
> is_glamor_pixmap is only true for FrontLeft, mind resending with this
> simplification?
Sure, the simplified version is as below. Thanks.
>From 6224fb9839f417cdd412667f4e6f502df872e7aa Mon Sep 17 00:00:00 2001
From: Zhigang Gong
Date: Fri, 27 Jul 2012 16:50:52 +0800
Subject: [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.
The previous implementation is to create a new textured
pixmap based on the newly created pixmap's buffer object.
This is not efficient, as we already created it when we
call CreatePixmap. We can just exchange the underlying
texture/image buffers by calling intel_glamor_exchange_buffers().
And this commit seems also fix a weird rendering problem
when working with compiz/mutter.
Signed-off-by: Zhigang Gong
---
src/intel_dri.c | 29 ++---
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/src/intel_dri.c b/src/intel_dri.c
index d027a64..fa1660c 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -110,6 +110,7 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable,
PixmapPtr pixmap)
{
ScreenPtr screen = drawable->pScreen;
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+ intel_screen_private *intel = intel_get_screen_private(scrn);
PixmapPtr old = get_drawable_pixmap(drawable);
struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
GCPtr gc;
@@ -139,28 +140,20 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable,
PixmapPtr pixmap)
}
intel_set_pixmap_private(pixmap, NULL);
- screen->DestroyPixmap(pixmap);
+ /* Exchange the underlying texture/image. */
+ intel_glamor_exchange_buffers(intel, old, pixmap);
/* And redirect the pixmap to the new bo (for 3D). */
intel_set_pixmap_private(old, priv);
old->refcnt++;
- /* This creating should not fail, as we already created its
-* successfully. But if it happens, we put a warning indicator
-* here, and the old pixmap will still be a glamor pixmap, and
-* latter the pixmap_flink will get a 0 name, then the X server
-* will pass a BadAlloc to the client.*/
- if (!intel_glamor_create_textured_pixmap(old))
- xf86DrvMsg(scrn->scrnIndex, X_WARNING,
- "Failed to get DRI drawable for glamor pixmap.\n");
-
screen->ModifyPixmapHeader(old,
drawable->width,
drawable->height,
0, 0,
priv->stride,
NULL);
-
+ screen->DestroyPixmap(pixmap);
intel_get_screen_private(xf86ScreenToScrn(screen))->needs_flush = TRUE;
return old;
}
@@ -194,10 +187,9 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
if (attachments[i] == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
- if (pixmap && intel_get_pixmap_private(pixmap) == NULL)
{
+ if (pixmap == NULL) {
+ drawable =
&(get_drawable_pixmap(drawable)->drawable);
is_glamor_pixmap = TRUE;
- drawable = &pixmap->drawable;
- pixmap = NULL;
}
} else if (attachments[i] == DRI2BufferStencil && pDepthPixmap)
{
pixmap = pDepthPixmap;
@@ -237,7 +229,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
goto unwind;
}
- if (attachment == DRI2BufferFrontLeft)
+ if (is_glamor_pixmap)
pixmap = fixup_glamor(drawable, pixmap);
}
@@ -314,10 +306,9 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int
attachment,
if (attachment == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
- if (pixmap && intel_get_pixmap_private(pixmap) == NULL) {
+ if (pixmap == NULL) {
+ drawable = &(get_drawable_pixmap(drawable)->drawable);
is_glamor_pixmap = TRUE;
- drawable = &pixmap->drawable;
- pixmap = NULL;
}
}
@@ -391,7 +382,7 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int
attachment,
free(buffer);
[Intel-gfx] [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.
From: Zhigang Gong
The previous implementation is to create a new textured
pixmap based on the newly created pixmap's buffer object.
This is not efficient, as we already created it when we
call CreatePixmap. We can just exchange the underlying
texture/image buffers by calling intel_glamor_exchange_buffers().
And this commit seems also fix a weird rendering problem
when working with compiz/mutter.
Signed-off-by: Zhigang Gong
---
src/intel_dri.c | 29 ++---
1 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/src/intel_dri.c b/src/intel_dri.c
index d027a64..c168b78 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -110,6 +110,7 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable,
PixmapPtr pixmap)
{
ScreenPtr screen = drawable->pScreen;
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+ intel_screen_private *intel = intel_get_screen_private(scrn);
PixmapPtr old = get_drawable_pixmap(drawable);
struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
GCPtr gc;
@@ -139,28 +140,20 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable,
PixmapPtr pixmap)
}
intel_set_pixmap_private(pixmap, NULL);
- screen->DestroyPixmap(pixmap);
+ /* Exchange the underlying texture/image. */
+ intel_glamor_exchange_buffers(intel, old, pixmap);
/* And redirect the pixmap to the new bo (for 3D). */
intel_set_pixmap_private(old, priv);
old->refcnt++;
- /* This creating should not fail, as we already created its
-* successfully. But if it happens, we put a warning indicator
-* here, and the old pixmap will still be a glamor pixmap, and
-* latter the pixmap_flink will get a 0 name, then the X server
-* will pass a BadAlloc to the client.*/
- if (!intel_glamor_create_textured_pixmap(old))
- xf86DrvMsg(scrn->scrnIndex, X_WARNING,
- "Failed to get DRI drawable for glamor pixmap.\n");
-
screen->ModifyPixmapHeader(old,
drawable->width,
drawable->height,
0, 0,
priv->stride,
NULL);
-
+ screen->DestroyPixmap(pixmap);
intel_get_screen_private(xf86ScreenToScrn(screen))->needs_flush = TRUE;
return old;
}
@@ -194,10 +187,9 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
if (attachments[i] == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
- if (pixmap && intel_get_pixmap_private(pixmap) == NULL)
{
+ if (pixmap == NULL) {
+ drawable =
&(get_drawable_pixmap(drawable)->drawable);
is_glamor_pixmap = TRUE;
- drawable = &pixmap->drawable;
- pixmap = NULL;
}
} else if (attachments[i] == DRI2BufferStencil && pDepthPixmap)
{
pixmap = pDepthPixmap;
@@ -237,7 +229,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
goto unwind;
}
- if (attachment == DRI2BufferFrontLeft)
+ if (attachment == DRI2BufferFrontLeft &&
is_glamor_pixmap)
pixmap = fixup_glamor(drawable, pixmap);
}
@@ -314,10 +306,9 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int
attachment,
if (attachment == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
- if (pixmap && intel_get_pixmap_private(pixmap) == NULL) {
+ if (pixmap == NULL) {
+ drawable = &(get_drawable_pixmap(drawable)->drawable);
is_glamor_pixmap = TRUE;
- drawable = &pixmap->drawable;
- pixmap = NULL;
}
}
@@ -391,7 +382,7 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int
attachment,
free(buffer);
return NULL;
}
- if (attachment == DRI2BufferFrontLeft)
+ if (attachment == DRI2BufferFrontLeft && is_glamor_pixmap)
pixmap = fixup_glamor(drawable, pixmap);
}
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] uxa-glyphs: Bypass uxa glyphs operations if using glamor.
From: Zhigang Gong
glamor_glyphs will never fallback. We don't need to keep a
uxa glyphs cache picture here. Thus simply bypass the
corresponding operations.
Signed-off-by: Zhigang Gong
---
uxa/uxa-glyphs.c | 12 +++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/uxa/uxa-glyphs.c b/uxa/uxa-glyphs.c
index 6bdf101..527942a 100644
--- a/uxa/uxa-glyphs.c
+++ b/uxa/uxa-glyphs.c
@@ -112,6 +112,9 @@ static void uxa_unrealize_glyph_caches(ScreenPtr pScreen)
uxa_screen_t *uxa_screen = uxa_get_screen(pScreen);
int i;
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR)
+ return;
+
if (!uxa_screen->glyph_cache_initialized)
return;
@@ -211,6 +214,11 @@ bail:
Bool uxa_glyphs_init(ScreenPtr pScreen)
{
+
+ uxa_screen_t *uxa_screen = uxa_get_screen(pScreen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR)
+ return TRUE;
#if HAS_DIXREGISTERPRIVATEKEY
if (!dixRegisterPrivateKey(&uxa_glyph_key, PRIVATE_GLYPH, 0))
return FALSE;
@@ -307,8 +315,10 @@ uxa_glyph_unrealize(ScreenPtr screen,
struct uxa_glyph *priv;
uxa_screen_t *uxa_screen = uxa_get_screen(screen);
- if (uxa_screen->info->flags & UXA_USE_GLAMOR)
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
glamor_glyph_unrealize(screen, glyph);
+ return;
+ }
/* Use Lookup in case we have not attached to this glyph. */
priv = dixLookupPrivate(&glyph->devPrivates, &uxa_glyph_key);
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] intel_dri: Fixed a buffer leak when enable glamor.
From: Zhigang Gong
We need to put current front_buffer to back buffer thus we
don't need to create a new back buffer next time. This behaviou
should be the same with or without glamor. Previous code
incorrectly discard the previous front_buffer and cause a
big buffer leak problem.
Signed-off-by: Zhigang Gong
---
src/intel_dri.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/intel_dri.c b/src/intel_dri.c
index ed5078e..0405937 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -1023,9 +1023,10 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
priv = info->front->driverPrivate;
/* Exchange the current front-buffer with the fresh bo */
+
+ intel->back_buffer = intel->front_buffer;
+ drm_intel_bo_reference(intel->back_buffer);
if (!(intel->uxa_flags & UXA_USE_GLAMOR)) {
- intel->back_buffer = intel->front_buffer;
- drm_intel_bo_reference(intel->back_buffer);
intel_set_pixmap_bo(priv->pixmap, new_back);
drm_intel_bo_unreference(new_back);
}
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] uxa/glamor/dri: Should fixup the drawable pixmap.
From: Zhigang Gong
Two fixes in this commit, first we only need to check the
front left buffer, for other attachment we don't need to
check them. The second is, we should fixup the pixmap's
drawable not the original drawable.
Signed-off-by: Zhigang Gong
---
src/intel_dri.c | 22 --
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/intel_dri.c b/src/intel_dri.c
index 2a0102d..f6f0c86 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -280,14 +280,18 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
pixmap = NULL;
if (attachments[i] == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
+
+ if (pixmap && intel_get_pixmap_private(pixmap) == NULL)
{
+ is_glamor_pixmap = TRUE;
+ drawable = &pixmap->drawable;
+ pixmap = NULL;
+ }
} else if (attachments[i] == DRI2BufferStencil && pDepthPixmap)
{
pixmap = pDepthPixmap;
pixmap->refcnt++;
}
- is_glamor_pixmap = pixmap && (intel_get_pixmap_private(pixmap)
== NULL);
-
- if (pixmap == NULL || is_glamor_pixmap) {
+ if (pixmap == NULL) {
unsigned int hint = INTEL_CREATE_PIXMAP_DRI2;
if (intel->tiling & INTEL_TILING_3D) {
@@ -398,11 +402,17 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int
attachment,
}
pixmap = NULL;
- if (attachment == DRI2BufferFrontLeft)
+ if (attachment == DRI2BufferFrontLeft) {
pixmap = get_front_buffer(drawable);
- is_glamor_pixmap = pixmap && (intel_get_pixmap_private(pixmap) == NULL);
- if (pixmap == NULL || is_glamor_pixmap) {
+ if (pixmap && intel_get_pixmap_private(pixmap) == NULL) {
+ is_glamor_pixmap = TRUE;
+ drawable = &pixmap->drawable;
+ pixmap = NULL;
+ }
+ }
+
+ if (pixmap == NULL) {
unsigned int hint = INTEL_CREATE_PIXMAP_DRI2;
int pixmap_width = drawable->width;
int pixmap_height = drawable->height;
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] uxa/glamor/dri: Enable the pageflip support on glamor.
From: Zhigang Gong
To support easy buffer exchange at glamor layer, glamor
added a new API glamor_egl_exchange_buffers() to exchange
two pixmaps' EGL image and fbos and textures without
recreating any of them. But this simple method's requirement
is that there are two pixmaps. A exceptional case is:
If we are using triple buffer when do page flipping, we
will have an extra back_buffer which doesn't have a pixmap
attached to it. Then each time we set that buffer to a
pixmap, we will have to call the create_egl_textured_pixmap
to create the corresponding EGL image and fbo and texture
for it. This is not efficient.
To fix this issue, this commit introduces a new back_pixmap
to intel structure to hold the back buffer and corresponding
glamor resources. Then we will just need to do the light
weight buffer exchanging at both DDX and glamor layer.
As the new back pixmap is similar to the screen pixmap
and need to be handled carefully when close screen. As the
glamor data structure is a per screen data, and will be
released at its close screen method. The glamor's close
screen method must cleanup the screen pixmap and back
pixmap's glamor resources. screen pixmap is easy to get,
but there is no good way to store the back pixmap.
So the glamor add a new API glamor_egl_create_textured_screen_ext
function to pass the back pixmap's pointer to glamor layer.
This commit make us depend on glamor commit: 4e58c4f.
And we increased the required glamor version from 0.3.0 to 0.3.1
Signed-off-by: Zhigang Gong
---
configure.ac|2 +-
src/intel.h |1 +
src/intel_display.c |8 +
src/intel_dri.c | 73 ++-
src/intel_driver.c |5 +++
src/intel_glamor.c | 19 ++---
src/intel_glamor.h |3 +-
7 files changed, 98 insertions(+), 13 deletions(-)
diff --git a/configure.ac b/configure.ac
index 785392a..1e77faf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,7 +158,7 @@ AC_ARG_ENABLE(glamor,
AC_MSG_RESULT([$GLAMOR])
AM_CONDITIONAL(GLAMOR, test x$GLAMOR != xno)
if test "x$GLAMOR" != "xno"; then
- PKG_CHECK_MODULES(LIBGLAMOR, [glamor >= 0.3.0])
+ PKG_CHECK_MODULES(LIBGLAMOR, [glamor >= 0.3.1])
PKG_CHECK_MODULES(LIBGLAMOR_EGL, [glamor-egl])
AC_DEFINE(USE_GLAMOR, 1, [Enable glamor acceleration])
fi
diff --git a/src/intel.h b/src/intel.h
index 16055e9..444cb6a 100644
--- a/src/intel.h
+++ b/src/intel.h
@@ -254,6 +254,7 @@ typedef struct intel_screen_private {
void *modes;
drm_intel_bo *front_buffer, *back_buffer;
+ PixmapPtr back_pixmap;
unsigned int back_name;
long front_pitch, front_tiling;
void *shadow_buffer;
diff --git a/src/intel_display.c b/src/intel_display.c
index d525ffa..868643a 100644
--- a/src/intel_display.c
+++ b/src/intel_display.c
@@ -47,6 +47,7 @@
#include "xf86DDC.h"
#include "intel_glamor.h"
+#include "uxa.h"
struct intel_mode {
int fd;
@@ -1369,6 +1370,7 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int
height)
int i, old_width, old_height, old_pitch;
unsigned long pitch;
uint32_t tiling;
+ ScreenPtr screen;
if (scrn->virtualX == width && scrn->virtualY == height)
return TRUE;
@@ -1382,6 +1384,12 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int
height)
old_fb_id = mode->fb_id;
old_front = intel->front_buffer;
+ if (intel->back_pixmap) {
+ screen = intel->back_pixmap->drawable.pScreen;
+ screen->DestroyPixmap(intel->back_pixmap);
+ intel->back_pixmap = NULL;
+ }
+
if (intel->back_buffer) {
drm_intel_bo_unreference(intel->back_buffer);
intel->back_buffer = NULL;
diff --git a/src/intel_dri.c b/src/intel_dri.c
index 98ae230..2a0102d 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -66,6 +66,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "dri2.h"
#include "intel_glamor.h"
+#include "uxa.h"
typedef struct {
int refcnt;
@@ -836,7 +837,7 @@ i830_dri2_del_frame_event(DrawablePtr drawable,
DRI2FrameEventPtr info)
}
static struct intel_pixmap *
-intel_exchange_pixmap_buffers(PixmapPtr front, PixmapPtr back)
+intel_exchange_pixmap_buffers(struct intel_screen_private *intel, PixmapPtr
front, PixmapPtr back)
{
struct intel_pixmap *new_front, *new_back;
@@ -847,6 +848,7 @@ intel_exchange_pixmap_buffers(PixmapPtr front, PixmapPtr
back)
new_front->busy = 1;
new_back->busy = -1;
+ intel_glamor_exchange_buffers(intel, front, back);
return new_front;
}
@@ -866,13 +868,46 @@ I830DRI2ExchangeBuffers(struct intel_screen_private
*intel, DRI2BufferPtr front,
back-&
[Intel-gfx] [PATCH 1/3] uxa/dri: Refine the pageflip processing.
From: Zhigang Gong
Add a new element back_name to intel structure to track
the back bo's name then avoid flink every time.
And at function I830DRI2ExchangeBuffers, after finish
the BO exchange between info's front and back pixmap,
it set the new front bo to the screen pixmap. But the
screen pixmap should be the same as front's pixmap,
so this is a duplicate operation and can be removed.
Signed-off-by: Zhigang Gong
---
src/intel.h |1 +
src/intel_dri.c | 48
2 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/src/intel.h b/src/intel.h
index 7593731..16055e9 100644
--- a/src/intel.h
+++ b/src/intel.h
@@ -254,6 +254,7 @@ typedef struct intel_screen_private {
void *modes;
drm_intel_bo *front_buffer, *back_buffer;
+ unsigned int back_name;
long front_pitch, front_tiling;
void *shadow_buffer;
int shadow_stride;
diff --git a/src/intel_dri.c b/src/intel_dri.c
index 8bc6157..98ae230 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -835,14 +835,27 @@ i830_dri2_del_frame_event(DrawablePtr drawable,
DRI2FrameEventPtr info)
free(info);
}
+static struct intel_pixmap *
+intel_exchange_pixmap_buffers(PixmapPtr front, PixmapPtr back)
+{
+ struct intel_pixmap *new_front, *new_back;
+
+ new_front = intel_get_pixmap_private(back);
+ new_back = intel_get_pixmap_private(front);
+ intel_set_pixmap_private(front, new_front);
+ intel_set_pixmap_private(back, new_back);
+ new_front->busy = 1;
+ new_back->busy = -1;
+
+ return new_front;
+}
+
static void
-I830DRI2ExchangeBuffers(DrawablePtr draw, DRI2BufferPtr front, DRI2BufferPtr
back)
+I830DRI2ExchangeBuffers(struct intel_screen_private *intel, DRI2BufferPtr
front, DRI2BufferPtr back)
{
I830DRI2BufferPrivatePtr front_priv, back_priv;
- struct intel_pixmap *front_intel, *back_intel;
- ScreenPtr screen;
- intel_screen_private *intel;
int tmp;
+ struct intel_pixmap *new_front;
front_priv = front->driverPrivate;
back_priv = back->driverPrivate;
@@ -853,21 +866,11 @@ I830DRI2ExchangeBuffers(DrawablePtr draw, DRI2BufferPtr
front, DRI2BufferPtr bac
back->name = tmp;
/* Swap pixmap bos */
- front_intel = intel_get_pixmap_private(front_priv->pixmap);
- back_intel = intel_get_pixmap_private(back_priv->pixmap);
- intel_set_pixmap_private(front_priv->pixmap, back_intel);
- intel_set_pixmap_private(back_priv->pixmap, front_intel);
-
- screen = draw->pScreen;
- intel = intel_get_screen_private(xf86Screens[screen->myNum]);
-
+ new_front = intel_exchange_pixmap_buffers(front_priv->pixmap,
+ back_priv->pixmap);
dri_bo_unreference (intel->front_buffer);
- intel->front_buffer = back_intel->bo;
+ intel->front_buffer = new_front->bo;
dri_bo_reference (intel->front_buffer);
-
- intel_set_pixmap_private(screen->GetScreenPixmap(screen), back_intel);
- back_intel->busy = 1;
- front_intel->busy = -1;
}
/*
@@ -881,6 +884,7 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
{
I830DRI2BufferPrivatePtr priv = info->back->driverPrivate;
drm_intel_bo *new_back, *old_back;
+ int tmp_name;
if (!intel->use_triple_buffer) {
if (!intel_do_pageflip(intel,
@@ -889,7 +893,7 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
return FALSE;
info->type = DRI2_SWAP;
- I830DRI2ExchangeBuffers(draw, info->front, info->back);
+ I830DRI2ExchangeBuffers(intel, info->front, info->back);
return TRUE;
}
@@ -915,6 +919,7 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
}
drm_intel_bo_disable_reuse(new_back);
+ dri_bo_flink(new_back, &intel->back_name);
} else {
new_back = intel->back_buffer;
intel->back_buffer = NULL;
@@ -934,10 +939,13 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
priv = info->front->driverPrivate;
intel_set_pixmap_bo(priv->pixmap, new_back);
- dri_bo_flink(new_back, &info->front->name);
+
+ tmp_name = info->front->name;
+ info->front->name = intel->back_name;
+ intel->back_name = tmp_name;
/* Then flip DRI2 pointers and update the screen pixmap */
- I830DRI2ExchangeBuffers(draw, info->front, info->back);
+ I830DRI2ExchangeBuffers(intel, info->front, info->back);
DRI2SwapComplete(info->client, draw, 0, 0, 0,
DRI2_EXCHANGE_COMPLETE,
info->event_complete,
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] uxa/glamor: Use a macro to specify module name.
> -Original Message-
> From: zhigang.g...@linux.intel.com [mailto:zhigang.g...@linux.intel.com]
> Sent: Thursday, February 02, 2012 11:31 AM
> To: ch...@chris-wilson.co.uk
> Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@linux.intel.com
> Subject: [PATCH] uxa/glamor: Use a macro to specify module name.
>
> From: Zhigang Gong
>
> Signed-off-by: Zhigang Gong
> ---
> src/intel_glamor.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/intel_glamor.c b/src/intel_glamor.c index
446dd3d..f908b7c
> 100644
> --- a/src/intel_glamor.c
> +++ b/src/intel_glamor.c
> @@ -67,7 +67,7 @@ intel_glamor_pre_init(ScrnInfoPtr scrn)
> intel = intel_get_screen_private(scrn);
>
> /* Load glamor module */
> - if ((glamor_module = xf86LoadSubModule(scrn, "glamor_egl"))) {
> + if ((glamor_module = xf86LoadSubModule(scrn,
> GLAMOR_EGL_MODULE_NAME)))
> +{
> version = xf86GetModuleVersion(glamor_module);
> if (version < MODULE_VERSION_NUMERIC(0,3,0)) {
> xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> --
> 1.7.4.4
This patch depends on glamor commit b5f8d...
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] uxa/glamor: Refine CloseScreen and InitScreen process.
> -Original Message-
> From:
> intel-gfx-bounces+zhigang.gong=linux.intel@lists.freedesktop.org
> [mailto:intel-gfx-bounces+zhigang.gong=linux.intel.com@lists.freedesktop.
> org] On Behalf Of Zhigang Gong
> Sent: Thursday, February 02, 2012 5:45 PM
> To: ch...@chris-wilson.co.uk
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] uxa/glamor: Refine CloseScreen and
> InitScreen process.
>
> > -Original Message-
> > From: zhigang.g...@linux.intel.com
> > [mailto:zhigang.g...@linux.intel.com]
> > Sent: Wednesday, February 01, 2012 7:47 PM
> > To: ch...@chris-wilson.co.uk
> > Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@linux.intel.com
> > Subject: [PATCH] uxa/glamor: Refine CloseScreen and InitScreen
> process.
> >
> > From: Zhigang Gong
> >
> > The previous version calls glamor_egl_close_screen and
> > glamor_egl_free_screen manually which is not align with standard
> process.
> > Now glamor change the way to follow standard method:
> >
> > glamor layer and glamor egl layer both have their internal CloseScreens.
> > The correct sequence is after the I830CloseScreen is registered, then
> > register glamor_egl_close_screen and the last one is
> > glamor_close_screen. So we move out the intel_glamor_init from the
> > intel_uxa_init to I830ScreenInit and just after the registration of
> > I830CloseScreen.
> >
> > As the glamor interfaces changed, we need to check the glamor version
> > when load the glamor egl module to make sure we are loading the right
> > glamor module. If failed, it will switch back to UXA path.
> >
> > Signed-off-by: Zhigang Gong
> > ---
> > src/intel_driver.c |3 +-
> > src/intel_glamor.c | 66
> > +++
> > src/intel_uxa.c|2 -
> > uxa/uxa.h | 10 ++-
> > 4 files changed, 44 insertions(+), 37 deletions(-)
> >
> > diff --git a/src/intel_driver.c b/src/intel_driver.c index
> 9d1c4e8..d66a8fd
> > 100644
> > --- a/src/intel_driver.c
> > +++ b/src/intel_driver.c
> > @@ -1051,6 +1051,7 @@ I830ScreenInit(int scrnIndex, ScreenPtr
> screen,
> > int argc, char **argv)
> > intel->CreateScreenResources = screen->CreateScreenResources;
> > screen->CreateScreenResources = i830CreateScreenResources;
> >
> > + intel_glamor_init(screen);
> > if (!xf86CrtcScreenInit(screen))
> > return FALSE;
> >
> > @@ -1124,8 +1125,6 @@ static void I830FreeScreen(int scrnIndex, int
> > flags)
> > ScrnInfoPtr scrn = xf86Screens[scrnIndex];
> > intel_screen_private *intel = intel_get_screen_private(scrn);
> >
> > - intel_glamor_free_screen(scrnIndex, flags);
> > -
> > if (intel) {
> > intel_mode_fini(intel);
> > intel_close_drm_master(intel);
> > diff --git a/src/intel_glamor.c b/src/intel_glamor.c index
> > e96daa6..446dd3d 100644
> > --- a/src/intel_glamor.c
> > +++ b/src/intel_glamor.c
> > @@ -61,20 +61,31 @@
> > intel_glamor_create_screen_resources(ScreenPtr screen) Bool
> > intel_glamor_pre_init(ScrnInfoPtr scrn) {
> > + pointer glamor_module;
> > intel_screen_private *intel;
> > + CARD32 version;
> > intel = intel_get_screen_private(scrn);
> >
> > /* Load glamor module */
> > - if (xf86LoadSubModule(scrn, "glamor_egl") &&
> > - glamor_egl_init(scrn, intel->drmSubFD)) {
> > - xf86DrvMsg(scrn->scrnIndex, X_INFO,
> > - "glamor detected, initialising\n");
> > - intel->uxa_flags |= UXA_USE_GLAMOR;
> > - } else {
> > + if ((glamor_module = xf86LoadSubModule(scrn, "glamor_egl"))) {
> > + version = xf86GetModuleVersion(glamor_module);
> > + if (version < MODULE_VERSION_NUMERIC(0,3,0)) {
> > + xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> > + "Incompatible glamor version, required >=
> 0.3.0.\n");
> > + }
> > + else {
> > + if (glamor_egl_init(scrn, intel->drmSubFD)) {
> > + xf86DrvMsg(scrn->scrnIndex, X_INFO,
> > + "glamor detected, initialising
> egl layer.\n");
> > + intel->uxa_flags =
> UXA_GLAMOR_EGL_INITIALIZED;
> > + }
> > + else
> > +
[Intel-gfx] [PATCH] uxa/glamor/dri: Fix a typo bug when fixup glamor pixmap.
From: Zhigang Gong Should modify the old pixmap's header not the new one which was already destroyed. Signed-off-by: Zhigang Gong --- src/intel_dri.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/intel_dri.c b/src/intel_dri.c index 0c6d45c..8bc6157 100644 --- a/src/intel_dri.c +++ b/src/intel_dri.c @@ -174,7 +174,7 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap) xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Failed to get DRI drawable for glamor pixmap.\n"); - screen->ModifyPixmapHeader(pixmap, + screen->ModifyPixmapHeader(old, drawable->width, drawable->height, 0, 0, -- 1.7.4.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] uxa/glamor: Refine CloseScreen and InitScreen process.
> -Original Message-
> From: zhigang.g...@linux.intel.com [mailto:zhigang.g...@linux.intel.com]
> Sent: Wednesday, February 01, 2012 7:47 PM
> To: ch...@chris-wilson.co.uk
> Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@linux.intel.com
> Subject: [PATCH] uxa/glamor: Refine CloseScreen and InitScreen process.
>
> From: Zhigang Gong
>
> The previous version calls glamor_egl_close_screen and
> glamor_egl_free_screen manually which is not align with standard process.
> Now glamor change the way to follow standard method:
>
> glamor layer and glamor egl layer both have their internal CloseScreens.
> The correct sequence is after the I830CloseScreen is registered, then
> register glamor_egl_close_screen and the last one is
> glamor_close_screen. So we move out the intel_glamor_init from the
> intel_uxa_init to I830ScreenInit and just after the registration of
> I830CloseScreen.
>
> As the glamor interfaces changed, we need to check the glamor version
> when load the glamor egl module to make sure we are loading the right
> glamor module. If failed, it will switch back to UXA path.
>
> Signed-off-by: Zhigang Gong
> ---
> src/intel_driver.c |3 +-
> src/intel_glamor.c | 66
> +++
> src/intel_uxa.c|2 -
> uxa/uxa.h | 10 ++-
> 4 files changed, 44 insertions(+), 37 deletions(-)
>
> diff --git a/src/intel_driver.c b/src/intel_driver.c index
9d1c4e8..d66a8fd
> 100644
> --- a/src/intel_driver.c
> +++ b/src/intel_driver.c
> @@ -1051,6 +1051,7 @@ I830ScreenInit(int scrnIndex, ScreenPtr screen,
> int argc, char **argv)
> intel->CreateScreenResources = screen->CreateScreenResources;
> screen->CreateScreenResources = i830CreateScreenResources;
>
> + intel_glamor_init(screen);
> if (!xf86CrtcScreenInit(screen))
> return FALSE;
>
> @@ -1124,8 +1125,6 @@ static void I830FreeScreen(int scrnIndex, int
> flags)
> ScrnInfoPtr scrn = xf86Screens[scrnIndex];
> intel_screen_private *intel = intel_get_screen_private(scrn);
>
> - intel_glamor_free_screen(scrnIndex, flags);
> -
> if (intel) {
> intel_mode_fini(intel);
> intel_close_drm_master(intel);
> diff --git a/src/intel_glamor.c b/src/intel_glamor.c index
> e96daa6..446dd3d 100644
> --- a/src/intel_glamor.c
> +++ b/src/intel_glamor.c
> @@ -61,20 +61,31 @@
> intel_glamor_create_screen_resources(ScreenPtr screen) Bool
> intel_glamor_pre_init(ScrnInfoPtr scrn) {
> + pointer glamor_module;
> intel_screen_private *intel;
> + CARD32 version;
> intel = intel_get_screen_private(scrn);
>
> /* Load glamor module */
> - if (xf86LoadSubModule(scrn, "glamor_egl") &&
> - glamor_egl_init(scrn, intel->drmSubFD)) {
> - xf86DrvMsg(scrn->scrnIndex, X_INFO,
> -"glamor detected, initialising\n");
> - intel->uxa_flags |= UXA_USE_GLAMOR;
> - } else {
> + if ((glamor_module = xf86LoadSubModule(scrn, "glamor_egl"))) {
> + version = xf86GetModuleVersion(glamor_module);
> + if (version < MODULE_VERSION_NUMERIC(0,3,0)) {
> + xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> + "Incompatible glamor version, required >=
0.3.0.\n");
> + }
> + else {
> + if (glamor_egl_init(scrn, intel->drmSubFD)) {
> + xf86DrvMsg(scrn->scrnIndex, X_INFO,
> +"glamor detected, initialising
egl layer.\n");
> + intel->uxa_flags =
UXA_GLAMOR_EGL_INITIALIZED;
> + }
> + else
> + xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> +"glamor detected, failed to
initialize egl.\n");
> + }
> + } else
> xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> "glamor not available\n");
> - intel->uxa_flags &= ~UXA_USE_GLAMOR;
> - }
>
> return TRUE;
> }
> @@ -83,7 +94,13 @@ PixmapPtr
> intel_glamor_create_pixmap(ScreenPtr screen, int w, int h,
> int depth, unsigned int usage)
> {
> - return glamor_create_pixmap(screen, w, h, depth, usage);
> + ScrnInfoPtr scrn = xf86Screens[screen->myNum];
> + intel_screen_private *intel = intel_get_screen_private(scrn);
> +
> + if (intel->uxa_flags & UXA_USE_GLAMOR)
> + ret
Re: [Intel-gfx] [PATCH] uxa/glamor: Use a macro to specify module name.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Thursday, February 02, 2012 4:52 PM > To: zhigang.g...@linux.intel.com > Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@linux.intel.com > Subject: Re: [PATCH] uxa/glamor: Use a macro to specify module name. > > On Thu, 2 Feb 2012 11:30:57 +0800, zhigang.g...@linux.intel.com > wrote: > > From: Zhigang Gong > > Would be good to mention which commit introduces the macro into > glamor, just to serve as a reminder that one needs to update. Same for the > other patches that introduces a new dependency. Agree, these patches should be reviewed with the corresponding glamor patches. For this patch, the glamor patch is as below: http://lists.freedesktop.org/archives/glamor/2012-February/68.html > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] uxa/glamor: Use a macro to specify module name.
From: Zhigang Gong
Signed-off-by: Zhigang Gong
---
src/intel_glamor.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 446dd3d..f908b7c 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -67,7 +67,7 @@ intel_glamor_pre_init(ScrnInfoPtr scrn)
intel = intel_get_screen_private(scrn);
/* Load glamor module */
- if ((glamor_module = xf86LoadSubModule(scrn, "glamor_egl"))) {
+ if ((glamor_module = xf86LoadSubModule(scrn, GLAMOR_EGL_MODULE_NAME))) {
version = xf86GetModuleVersion(glamor_module);
if (version < MODULE_VERSION_NUMERIC(0,3,0)) {
xf86DrvMsg(scrn->scrnIndex, X_ERROR,
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] uxa/glamor: Refine CloseScreen and InitScreen process.
From: Zhigang Gong
The previous version calls glamor_egl_close_screen and
glamor_egl_free_screen manually which is not align with
standard process. Now glamor change the way to follow
standard method:
glamor layer and glamor egl layer both have their internal
CloseScreens. The correct sequence is after the I830CloseScreen
is registered, then register glamor_egl_close_screen and
the last one is glamor_close_screen. So we move out the
intel_glamor_init from the intel_uxa_init to I830ScreenInit
and just after the registration of I830CloseScreen.
As the glamor interfaces changed, we need to check the
glamor version when load the glamor egl module to make
sure we are loading the right glamor module. If
failed, it will switch back to UXA path.
Signed-off-by: Zhigang Gong
---
src/intel_driver.c |3 +-
src/intel_glamor.c | 66 +++
src/intel_uxa.c|2 -
uxa/uxa.h | 10 ++-
4 files changed, 44 insertions(+), 37 deletions(-)
diff --git a/src/intel_driver.c b/src/intel_driver.c
index 9d1c4e8..d66a8fd 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -1051,6 +1051,7 @@ I830ScreenInit(int scrnIndex, ScreenPtr screen, int argc,
char **argv)
intel->CreateScreenResources = screen->CreateScreenResources;
screen->CreateScreenResources = i830CreateScreenResources;
+ intel_glamor_init(screen);
if (!xf86CrtcScreenInit(screen))
return FALSE;
@@ -1124,8 +1125,6 @@ static void I830FreeScreen(int scrnIndex, int flags)
ScrnInfoPtr scrn = xf86Screens[scrnIndex];
intel_screen_private *intel = intel_get_screen_private(scrn);
- intel_glamor_free_screen(scrnIndex, flags);
-
if (intel) {
intel_mode_fini(intel);
intel_close_drm_master(intel);
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index e96daa6..446dd3d 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -61,20 +61,31 @@ intel_glamor_create_screen_resources(ScreenPtr screen)
Bool
intel_glamor_pre_init(ScrnInfoPtr scrn)
{
+ pointer glamor_module;
intel_screen_private *intel;
+ CARD32 version;
intel = intel_get_screen_private(scrn);
/* Load glamor module */
- if (xf86LoadSubModule(scrn, "glamor_egl") &&
- glamor_egl_init(scrn, intel->drmSubFD)) {
- xf86DrvMsg(scrn->scrnIndex, X_INFO,
- "glamor detected, initialising\n");
- intel->uxa_flags |= UXA_USE_GLAMOR;
- } else {
+ if ((glamor_module = xf86LoadSubModule(scrn, "glamor_egl"))) {
+ version = xf86GetModuleVersion(glamor_module);
+ if (version < MODULE_VERSION_NUMERIC(0,3,0)) {
+ xf86DrvMsg(scrn->scrnIndex, X_ERROR,
+ "Incompatible glamor version, required >= 0.3.0.\n");
+ }
+ else {
+ if (glamor_egl_init(scrn, intel->drmSubFD)) {
+ xf86DrvMsg(scrn->scrnIndex, X_INFO,
+ "glamor detected, initialising egl
layer.\n");
+ intel->uxa_flags = UXA_GLAMOR_EGL_INITIALIZED;
+ }
+ else
+ xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+ "glamor detected, failed to
initialize egl.\n");
+ }
+ } else
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
"glamor not available\n");
- intel->uxa_flags &= ~UXA_USE_GLAMOR;
- }
return TRUE;
}
@@ -83,7 +94,13 @@ PixmapPtr
intel_glamor_create_pixmap(ScreenPtr screen, int w, int h,
int depth, unsigned int usage)
{
- return glamor_create_pixmap(screen, w, h, depth, usage);
+ ScrnInfoPtr scrn = xf86Screens[screen->myNum];
+ intel_screen_private *intel = intel_get_screen_private(scrn);
+
+ if (intel->uxa_flags & UXA_USE_GLAMOR)
+ return glamor_create_pixmap(screen, w, h, depth, usage);
+ else
+ return NULL;
}
Bool
@@ -145,17 +162,16 @@ intel_glamor_finish_access(PixmapPtr pixmap, uxa_access_t
access)
return;
}
-
Bool
intel_glamor_init(ScreenPtr screen)
{
ScrnInfoPtr scrn = xf86Screens[screen->myNum];
intel_screen_private *intel = intel_get_screen_private(scrn);
- if ((intel->uxa_flags & UXA_USE_GLAMOR) == 0)
- return TRUE;
+ if ((intel->uxa_flags & UXA_GLAMOR_EGL_INITIALIZED) == 0)
+ goto fail;
- if (!glamor_init(screen, GLAMOR_INVERTED_Y_AXIS)) {
+ if (!glamor_init(screen, GLAMOR_INVERTED_Y_AXIS |
GLAMOR_USE_EGL_SCREEN)) {
[Intel-gfx] [PATCH v3] uxa/glamor: Create glamor pixmap by default.
From: Zhigang Gong
Chris,
According to your previous comments. The X server will not die when
fail to get a dri buffer for a glamor pixmap, instead it will return
a BadAlloc. Your previous advice is to pass a BadMatch to the client,
and I traced into X's dri2 module and found current code can only
return a BadAlloc. IMHO, this should be ok. Right?
Another change is to disable the reuse of bo for glamor textured
pixmap.
The last change is reduce the performance gain. ;), I just found
another 10-20% is come from my latest glamor patchset.
---
As create glamor pixmap by default will get much better performance
by using the textured-drm pixmap, this commit is to make that the
default behaviour when configure to use glamor. Another advantage
of this commit is that we reduce the risk of encountering the
"incompatible region exists for this name" and the associated
render corruption. And since we now never intentionally allocate
a reusable pixmap we could just make all (intel_glamor) allocations
non-reusable without incurring too great an overhead.
A side effect is that for those glamor pixmaps, they don't have a
valid BO attached to it and thus it fails to get a DRI drawable. This
commit also fixes that problem by coping the fixup_shadow mechanism. I
tested this with mutter, and it works fine.
The performance gain to apply this patch is about 10% to 20% with
different workload.
Signed-off-by: Zhigang Gong
---
src/intel_dri.c| 89 +++
src/intel_glamor.c |2 +
src/intel_uxa.c|6 +++
3 files changed, 90 insertions(+), 7 deletions(-)
diff --git a/src/intel_dri.c b/src/intel_dri.c
index df3338f..f0494c2 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -66,6 +66,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "dri2.h"
+#include "intel_glamor.h"
+
typedef struct {
int refcnt;
PixmapPtr pixmap;
@@ -125,6 +127,65 @@ static PixmapPtr get_front_buffer(DrawablePtr drawable)
return pixmap;
}
+static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
+{
+ ScreenPtr screen = drawable->pScreen;
+ ScrnInfoPtr scrn = xf86Screens[screen->myNum];
+ PixmapPtr old = get_drawable_pixmap(drawable);
+ struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
+ GCPtr gc;
+
+ /* With a glamor pixmap, 2D pixmaps are created in texture
+* and without a static BO attached to it. To support DRI,
+* we need to create a new textured-drm pixmap and
+* need to copy the original content to this new textured-drm
+* pixmap, and then convert the old pixmap to a coherent
+* textured-drm pixmap which has a valid BO attached to it
+* and also has a valid texture, thus both glamor and DRI2
+* can access it.
+*
+*/
+
+ /* Copy the current contents of the pixmap to the bo. */
+ gc = GetScratchGC(drawable->depth, screen);
+ if (gc) {
+ ValidateGC(&pixmap->drawable, gc);
+ gc->ops->CopyArea(drawable, &pixmap->drawable,
+ gc,
+ 0, 0,
+ drawable->width,
+ drawable->height,
+ 0, 0);
+ FreeScratchGC(gc);
+ }
+
+ intel_set_pixmap_private(pixmap, NULL);
+ screen->DestroyPixmap(pixmap);
+
+ /* And redirect the pixmap to the new bo (for 3D). */
+ intel_set_pixmap_private(old, priv);
+ old->refcnt++;
+
+ /* This creating should not fail, as we already created its
+* successfully. But if it happens, we put a warning indicator
+* here, and the old pixmap will still be a glamor pixmap, and
+* latter the pixmap_flink will get a 0 name, then the X server
+* will pass a BadAlloc to the client.*/
+ if (!intel_glamor_create_textured_pixmap(old))
+ xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+ "Failed to get DRI drawable for glamor pixmap.\n");
+
+ screen->ModifyPixmapHeader(pixmap,
+ drawable->width,
+ drawable->height,
+ 0, 0,
+ priv->stride,
+ NULL);
+
+ intel_get_screen_private(xf86Screens[screen->myNum])->needs_flush =
TRUE;
+ return old;
+}
+
static PixmapPtr fixup_shadow(DrawablePtr drawable, PixmapPtr pixmap)
{
ScreenPtr screen = drawable->pScreen;
@@ -203,6 +264,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
I830DRI2BufferPrivatePtr privates;
PixmapPtr pixmap, pDepthPixmap;
int i;
+ int is_glamor_pixmap;
buffers = calloc(co
Re: [Intel-gfx] [Glamor] [PATCH v2] uxa/glamor: Create glamor pixmap by default.
> -Original Message- > From: > glamor-bounces+zhigang.gong=linux.intel@lists.freedesktop.org > [mailto:glamor-bounces+zhigang.gong=linux.intel.com@lists.freedesktop.o > rg] On Behalf Of Chris Wilson > Sent: Wednesday, January 11, 2012 2:01 AM > To: zhigang.g...@gmail.com > Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@linux.intel.com; > zhigang.g...@gmail.com; gla...@lists.freedesktop.org > Subject: Re: [Glamor] [PATCH v2] uxa/glamor: Create glamor pixmap by > default. > > On Wed, 11 Jan 2012 10:01:09 +0800, zhigang.g...@gmail.com wrote: > > From: Zhigang Gong > > > > A minor fix, after convert the old pixmap to the textured-drm pixmap, > > we need to modify the old pixmap's header to make sure the > > width/height and stride are the same as the new textured-drm BO. > > We can not just call FatalError there, but should just propagate the error > so that the client at least sees BadMatch and X doesn't simply die... How Agree. X server should not die for a unsupported client application. Maybe I can just change it to put a warning indicator there. As to propagate the error to client, I have no idea how to pass a BadMatch error To client. Just return the old texture_only pixmap, and the pixmap_flink will get a 0 name, and then the client will get a 0 buffer name. If the client check the buffer name then it can find something bad happened. If it doesn't, then client may trigger a segfault which is what mutter does currently. Any hint here? > does this resolve the issue of mesa replacing a bo for a glamor pixmap > even though we've exported it? Or is that simply an orthogonal problem to > be tackled later? My understanding is for the texture which is bound to KHR Image by using EGLlImageTargetTexture2DOES, mesa will not replacing its BO to a new one. For those pure glamor pixmap, as we never export a pure glamor pixmap's BO, it will not be a problem. The fixup_glamor will convert a texture only pixmap to a textured_drm pixmap eventually, then after we export its BO, mesa will not change it. Right? > > > --- > > As create glamor pixmap by default will get much better performance by > > using the textured-drm pixmap, this commit is to make that the default > > behaviour when configure to use glamor. > > > > A side effect is that for those glamor pixmaps, they don't have a > > valid BO attached to it and thus it fails to get a DRI drawable. This > > commit also fixes that problem by copy the fixup_shadow mechanism. I > > tested this with mutter, and it seems work fine. > > > > The performance gain to apply this patch is about 20% to 40% with > > different workload. > > Waiting to see if I get any results to support that claim... ;-) You can also > mention that by using glamor to allocate the pixmaps, we reduce the risk > of encountering the "incompatible region exists for this name" > and the associated render corruption. Until that is resolved, every time we > export a DRI pixmap and create a BO we still may trigger that bug. Yeah, will add that at v3 patch. > However, since we now never intentionally allocate a reusable pixmap we > could just make all (intel_glamor) allocations non-reusable without > incurring too great an overhead. Agree, will disable reusable when create textured_drm pixmap at v3 patch. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Glamor mailing list > gla...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/glamor ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] uxa/glamor: Create glamor pixmap by default.
From: Zhigang Gong
A minor fix, after convert the old pixmap to the textured-drm pixmap,
we need to modify the old pixmap's header to make sure the width/height
and stride are the same as the new textured-drm BO.
---
As create glamor pixmap by default will get much better performance
by using the textured-drm pixmap, this commit is to make that the
default behaviour when configure to use glamor.
A side effect is that for those glamor pixmaps, they don't have a
valid BO attached to it and thus it fails to get a DRI drawable. This
commit also fixes that problem by copy the fixup_shadow mechanism. I
tested this with mutter, and it seems work fine.
The performance gain to apply this patch is about 20% to 40% with
different workload.
Signed-off-by: Zhigang Gong
---
src/intel_dri.c | 83 ++
src/intel_uxa.c |6
2 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/intel_dri.c b/src/intel_dri.c
index df3338f..8a25ae8 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -66,6 +66,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "dri2.h"
+#include "intel_glamor.h"
+
typedef struct {
int refcnt;
PixmapPtr pixmap;
@@ -125,6 +127,59 @@ static PixmapPtr get_front_buffer(DrawablePtr drawable)
return pixmap;
}
+static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
+{
+ ScreenPtr screen = drawable->pScreen;
+ PixmapPtr old = get_drawable_pixmap(drawable);
+ struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
+ GCPtr gc;
+
+ /* With a glamor pixmap, 2D pixmaps are created in texture
+* and without a static BO attached to it. To support DRI,
+* we need to create a new textured-drm pixmap and
+* need to copy the original content to this new textured-drm
+* pixmap, and then convert the old pixmap to a coherent
+* textured-drm pixmap which has a valid BO attached to it
+* and also has a valid texture, thus both glamor and DRI2
+* can access it.
+*
+*/
+
+ /* Copy the current contents of the pixmap to the bo. */
+ gc = GetScratchGC(drawable->depth, screen);
+ if (gc) {
+ ValidateGC(&pixmap->drawable, gc);
+ gc->ops->CopyArea(drawable, &pixmap->drawable,
+ gc,
+ 0, 0,
+ drawable->width,
+ drawable->height,
+ 0, 0);
+ FreeScratchGC(gc);
+ }
+
+ intel_set_pixmap_private(pixmap, NULL);
+ screen->DestroyPixmap(pixmap);
+
+ /* And redirect the pixmap to the new bo (for 3D). */
+ intel_set_pixmap_private(old, priv);
+ old->refcnt++;
+
+ if (!intel_glamor_create_textured_pixmap(old)) {
+ FatalError("Failed to get DRI drawable for glamor pixmap.\n");
+ }
+
+ screen->ModifyPixmapHeader(pixmap,
+ drawable->width,
+ drawable->height,
+ 0, 0,
+ priv->stride,
+ NULL);
+
+ intel_get_screen_private(xf86Screens[screen->myNum])->needs_flush =
TRUE;
+ return old;
+}
+
static PixmapPtr fixup_shadow(DrawablePtr drawable, PixmapPtr pixmap)
{
ScreenPtr screen = drawable->pScreen;
@@ -203,6 +258,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
I830DRI2BufferPrivatePtr privates;
PixmapPtr pixmap, pDepthPixmap;
int i;
+ int is_glamor_pixmap;
buffers = calloc(count, sizeof *buffers);
if (buffers == NULL)
@@ -222,7 +278,10 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
pixmap = pDepthPixmap;
pixmap->refcnt++;
}
- if (pixmap == NULL) {
+
+ is_glamor_pixmap = pixmap && (intel_get_pixmap_private(pixmap)
== NULL);
+
+ if (pixmap == NULL || is_glamor_pixmap) {
unsigned int hint = INTEL_CREATE_PIXMAP_DRI2;
if (intel->tiling & INTEL_TILING_3D) {
@@ -255,8 +314,12 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
goto unwind;
}
- if (attachment == DRI2BufferFrontLeft)
- pixmap = fixup_shadow(drawable, pixmap);
+ if (attachment == DRI2BufferFrontLeft) {
+ if (!is_glamor_pixmap)
+ pixmap = fixup_shadow(drawable, pixmap)
[Intel-gfx] [PATCH] uxa/glamor: Create glamor pixmap by default.
From: Zhigang Gong
As create glamor pixmap by default will get much better performance
by using the textured-drm pixmap, this commit is to make that the
default behaviour when configure to use glamor.
A side effect is that for those glamor pixmaps, they don't have a
valid BO attached to it and thus it fails to get a DRI drawable. This
commit also fixes that problem by copy the fixup_shadow mechanism. I
tested this with mutter, and it seems work fine.
The performance gain to apply this patch is about 20% to 40% with
different workload.
Signed-off-by: Zhigang Gong
---
src/intel_dri.c | 76 +-
src/intel_uxa.c |6
2 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/src/intel_dri.c b/src/intel_dri.c
index df3338f..c9444c9 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -66,6 +66,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "dri2.h"
+#include "intel_glamor.h"
+
typedef struct {
int refcnt;
PixmapPtr pixmap;
@@ -125,6 +127,52 @@ static PixmapPtr get_front_buffer(DrawablePtr drawable)
return pixmap;
}
+static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
+{
+ ScreenPtr screen = drawable->pScreen;
+ PixmapPtr old = get_drawable_pixmap(drawable);
+ struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
+ GCPtr gc;
+
+ /* With a glamor pixmap, 2D pixmaps are created in texture
+* and without a static BO attached to it. To support DRI,
+* we need to create a new textured-drm pixmap and
+* need to copy the original content to this new textured-drm
+* pixmap, and then convert the old pixmap to a coherent
+* textured-drm pixmap which has a valid BO attached to it
+* and also has a valid texture, thus both glamor and DRI2
+* can access it.
+*
+*/
+
+ /* Copy the current contents of the pixmap to the bo. */
+ gc = GetScratchGC(drawable->depth, screen);
+ if (gc) {
+ ValidateGC(&pixmap->drawable, gc);
+ gc->ops->CopyArea(drawable, &pixmap->drawable,
+ gc,
+ 0, 0,
+ drawable->width,
+ drawable->height,
+ 0, 0);
+ FreeScratchGC(gc);
+ }
+
+ intel_set_pixmap_private(pixmap, NULL);
+ screen->DestroyPixmap(pixmap);
+
+ /* And redirect the pixmap to the new bo (for 3D). */
+ intel_set_pixmap_private(old, priv);
+ old->refcnt++;
+
+ if (!intel_glamor_create_textured_pixmap(old)) {
+ FatalError("Failed to get DRI drawable for glamor pixmap.\n");
+ }
+
+ intel_get_screen_private(xf86Screens[screen->myNum])->needs_flush =
TRUE;
+ return old;
+}
+
static PixmapPtr fixup_shadow(DrawablePtr drawable, PixmapPtr pixmap)
{
ScreenPtr screen = drawable->pScreen;
@@ -203,6 +251,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
I830DRI2BufferPrivatePtr privates;
PixmapPtr pixmap, pDepthPixmap;
int i;
+ int is_glamor_pixmap;
buffers = calloc(count, sizeof *buffers);
if (buffers == NULL)
@@ -222,7 +271,10 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
pixmap = pDepthPixmap;
pixmap->refcnt++;
}
- if (pixmap == NULL) {
+
+ is_glamor_pixmap = pixmap && (intel_get_pixmap_private(pixmap)
== NULL);
+
+ if (pixmap == NULL || is_glamor_pixmap) {
unsigned int hint = INTEL_CREATE_PIXMAP_DRI2;
if (intel->tiling & INTEL_TILING_3D) {
@@ -255,8 +307,12 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int
*attachments,
goto unwind;
}
- if (attachment == DRI2BufferFrontLeft)
- pixmap = fixup_shadow(drawable, pixmap);
+ if (attachment == DRI2BufferFrontLeft) {
+ if (!is_glamor_pixmap)
+ pixmap = fixup_shadow(drawable, pixmap);
+ else
+ pixmap = fixup_glamor(drawable, pixmap);
+ }
}
if (attachments[i] == DRI2BufferDepth)
@@ -317,6 +373,7 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int
attachment,
DRI2Buffer2Ptr buffer;
I830DRI2BufferPrivatePtr privates;
PixmapPtr pixmap;
+ int is_glamor_pixmap;
buffer = calloc(1, sizeof *buffer);
if (buffer == NULL)
@@ -330,7 +387,9 @@ I8
Re: [Intel-gfx] [PATCH 0/1] uxa/glamor: Route some drawing function to glamor.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Wednesday, January 04, 2012 5:47 AM > To: zhigang.g...@linux.intel.com > Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com; > zhigang.g...@linux.intel.com > Subject: Re: [PATCH 0/1] uxa/glamor: Route some drawing function to > glamor. > > On Sat, 31 Dec 2011 21:18:14 +0800, zhigang.g...@linux.intel.com > wrote: > > From: Zhigang Gong > > > > I agree to pending the last patch which is to create glamor pixmap by > default. > > This patch is based on the other 3 patch. After apply this patch, and > > use the latest glamor. > > I've pushed those 4 patches (all bar to use > intel_glamor_create_textured_pixmap by default). I've a new segfault for > you in glamor_core.c::glamor_validate_gc(), line 425 as pixmap_priv is > NULL, which I only hit just as I thought I had completed testing the code. > > Afaics, the remaining big topics for the ddx are how to enable glx and > integration between glamor/uxa render paths, right? Exactly. That's the hardest part now. There are two DRI loaders, one is AIGLX and the other is EGL. Both loader have their own glapi.c. Currently, the AIGLX always get loaded before the DDX driver, and thus the glapi in glx will be used. One possible solution is as below, please help to review and comment: let the glamor egl module get loaded prior to the AIGLX And load the proper libGL.so thus the egl will create a complete and correct context. And then latter, we can call _glapi_create_table_from_handle to initialize the glx's dispatch structure. We can treat the context created by egl for glamor is the context dedicated for server client, and each time when call into glamor's rendering path, force current to glamor's context and restore to previous context at returning. Copy to KRH, I know you are the master of DRI2/AIGLX. Any suggestion here? Thanks. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] uxa/glamor: Route some missing drawing function to glamor.
From: Zhigang Gong
We have to route all the drawing function to glamor at first
if glamor is enabled. The previous commit missed some function.
Now add them in.
Signed-off-by: Zhigang Gong
---
uxa/uxa-accel.c | 142 --
uxa/uxa-glamor.h |8 +++
2 files changed, 146 insertions(+), 4 deletions(-)
diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index 00948b7..d67c1a6 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -558,6 +558,18 @@ uxa_poly_point(DrawablePtr pDrawable, GCPtr pGC, int mode,
int npt,
{
int i;
xRectangle *prect;
+ uxa_screen_t *uxa_screen = uxa_get_screen(pDrawable->pScreen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ int ok;
+
+ uxa_prepare_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ ok = glamor_poly_point_nf(pDrawable, pGC, mode, npt, ppt);
+ uxa_finish_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+
+ if (ok)
+ return;
+ }
/* If we can't reuse the current GC as is, don't bother accelerating the
* points.
@@ -596,6 +608,18 @@ uxa_poly_lines(DrawablePtr pDrawable, GCPtr pGC, int mode,
int npt,
xRectangle *prect;
int x1, x2, y1, y2;
int i;
+ uxa_screen_t *uxa_screen = uxa_get_screen(pDrawable->pScreen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ int ok;
+
+ uxa_prepare_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ ok = glamor_poly_lines_nf(pDrawable, pGC, mode, npt, ppt);
+ uxa_finish_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+
+ if (ok)
+ return;
+ }
/* Don't try to do wide lines or non-solid fill style. */
if (pGC->lineWidth != 0 || pGC->lineStyle != LineSolid ||
@@ -657,6 +681,18 @@ uxa_poly_segment(DrawablePtr pDrawable, GCPtr pGC, int
nseg, xSegment * pSeg)
{
xRectangle *prect;
int i;
+ uxa_screen_t *uxa_screen = uxa_get_screen(pDrawable->pScreen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ int ok;
+
+ uxa_prepare_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ ok = glamor_poly_segment_nf(pDrawable, pGC, nseg, pSeg);
+ uxa_finish_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+
+ if (ok)
+ return;
+ }
/* Don't try to do wide lines or non-solid fill style. */
if (pGC->lineWidth != 0 || pGC->lineStyle != LineSolid ||
@@ -889,12 +925,110 @@ fallback:
uxa_check_set_spans(pDrawable, gc, src, points, widths, n, sorted);
}
+static RegionPtr
+uxa_copy_plane(DrawablePtr pSrc, DrawablePtr pDst, GCPtr pGC,
+ int srcx, int srcy, int w, int h, int dstx, int dsty,
+ unsigned long bitPlane)
+{
+ ScreenPtr screen = pDst->pScreen;
+ uxa_screen_t *uxa_screen = uxa_get_screen(screen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ int ok;
+ RegionPtr region;
+
+ uxa_prepare_access(pDst, UXA_GLAMOR_ACCESS_RW);
+ uxa_prepare_access(pSrc, UXA_GLAMOR_ACCESS_RO);
+ ok = glamor_copy_plane_nf(pSrc, pDst, pGC, srcx, srcy, w, h,
+ dstx, dsty, bitPlane, ®ion);
+ uxa_finish_access(pSrc, UXA_GLAMOR_ACCESS_RO);
+ uxa_finish_access(pDst, UXA_GLAMOR_ACCESS_RW);
+ if (!ok)
+ goto fallback;
+ return region;
+ }
+
+fallback:
+ return uxa_check_copy_plane(pSrc, pDst, pGC, srcx, srcy, w, h,
+ dstx, dsty, bitPlane);
+}
+
+static void
+uxa_image_glyph_blt(DrawablePtr pDrawable, GCPtr pGC,
+ int x, int y, unsigned int nglyph,
+ CharInfoPtr * ppci, pointer pglyphBase)
+{
+ ScreenPtr screen = pDrawable->pScreen;
+ uxa_screen_t *uxa_screen = uxa_get_screen(screen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ int ok;
+
+ uxa_prepare_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ ok = glamor_image_glyph_blt_nf(pDrawable, pGC, x, y, nglyph,
ppci, pglyphBase);
+ uxa_finish_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ if (!ok)
+ goto fallback;
+ return;
+ }
+
+fallback:
+ uxa_check_image_glyph_blt(pDrawable, pGC, x, y, nglyph, ppci,
pglyphBase);
+}
+
+static void
+uxa_poly_glyph_blt(DrawablePtr pDrawable, GCPtr pGC,
+ int x, int y, unsigned int nglyph,
+ CharInfoPtr * ppci, pointer pglyphBase)
+{
+ ScreenPtr screen = pDrawable->pScreen;
+ uxa_screen_t *uxa_screen = uxa_get_screen(screen);
+
+ if (uxa_
[Intel-gfx] [PATCH 0/1] uxa/glamor: Route some drawing function to glamor.
From: Zhigang Gong I agree to pending the last patch which is to create glamor pixmap by default. This patch is based on the other 3 patch. After apply this patch, and use the latest glamor. I spent too much time to fix those XTS regressions this week and haven't have a chance to add version information to it. Will do that after the vacation. The good news is that now uxa/glamor has no regression compare to UXA. Signed-off-by: Zhigang Gong - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] uxa/glamor: Create glamor pixmap by default.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Wednesday, December 28, 2011 10:27 PM > To: zhigang.g...@linux.intel.com > Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com; > zhigang.g...@linux.intel.com > Subject: Re: [PATCH 4/4] uxa/glamor: Create glamor pixmap by default. > > On Tue, 27 Dec 2011 17:09:18 +0800, zhigang.g...@linux.intel.com > wrote: > > From: Zhigang Gong > > > > As a pure glamor pixmap has a local texture rather than bind a pixmap > > to a external BO. This can avoid some unecessary flush, and can > > achieve better performance. > > The testing on my machine shows that aa10text/rgb10text get about > > 20-30% performance improvement. > > How do we get the name back for a pixmap created by glamor and then > attached to a DRI2Drawable? And then once that name is exposed, how do > you prevent GL from recreating the bo attached to the pixmap? You are right, this patch will break the DRI2 usage. And currently, one solution come to my mind: At I830DRI2Create buffer, After we get a non-null pixmap at get_front_buffer we check whether The pixmap has a valid bo, if it doesn't, we will think this is a pure glamor pixmap (maybe we can check more fields, such as zero devPrivate.ptr) Then we will create new a valid DRI2 pixmap, then we call fixup_glamor to unbind the bo from the new DRI2 pixmap and then bind it to the old pure glamor pixmap and call intel_create_textured_pixmap to create a coherent glamor-drm pixmap, the old pixmap's pointer will not change, only the type will be changed to a glamor-drm and has a valid bo attached to it. Does this way work? And will it bring too much overhead. Base on my current testing, change to use pure glamor pixmap for non-dri2 pixmap really get much better performance then use coherent glamor-drm pixmap. Any comments？ > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] uxa/glamor: Create glamor pixmap by default.
From: Zhigang Gong
As a pure glamor pixmap has a local texture rather than
bind a pixmap to a external BO. This can avoid some
unecessary flush, and can achieve better performance.
The testing on my machine shows that aa10text/rgb10text
get about 20-30% performance improvement.
Signed-off-by: Zhigang Gong
---
src/intel_uxa.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 9d74554..ca3220a 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -1026,6 +1026,11 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
struct intel_pixmap *priv;
PixmapPtr pixmap, new_pixmap = NULL;
+ if (!(usage & INTEL_CREATE_PIXMAP_DRI2)) {
+ pixmap = intel_glamor_create_pixmap(screen, w, h, depth, usage);
+ if (pixmap)
+ return pixmap;
+ }
if (w > 32767 || h > 32767)
return NullPixmap;
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] uxa/glamor: Let glamor do the GC validation.
From: Zhigang Gong
If we are using GLAMOR, then a tile pixmap or stipple pixmap
may be pure glamor pixmap and thus UXA doesn't know how to
render to them, we should let glamor to do the validation.
Signed-off-by: Zhigang Gong
---
uxa/uxa-glamor.h |1 +
uxa/uxa.c| 12
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/uxa/uxa-glamor.h b/uxa/uxa-glamor.h
index 71a9c29..2b4b452 100644
--- a/uxa/uxa-glamor.h
+++ b/uxa/uxa-glamor.h
@@ -51,6 +51,7 @@
#define glamor_trapezoids_nf(...) FALSE
#define glamor_triangles_nf(...) FALSE
#define glamor_add_traps_nf(...) FALSE
+#define glamor_create_gc(...) FALSE
#endif
#endif /* UXA_GLAMOR_H */
diff --git a/uxa/uxa.c b/uxa/uxa.c
index 5b3d709..56bc233 100644
--- a/uxa/uxa.c
+++ b/uxa/uxa.c
@@ -38,6 +38,7 @@
#include
#include "dixfontstr.h"
#include "uxa.h"
+#include "uxa-glamor.h"
#if HAS_DEVPRIVATEKEYREC
DevPrivateKeyRec uxa_screen_index;
@@ -183,11 +184,21 @@ void uxa_finish_access(DrawablePtr pDrawable,
uxa_access_t access)
static void
uxa_validate_gc(GCPtr pGC, unsigned long changes, DrawablePtr pDrawable)
{
+ uxa_screen_t *uxa_screen = uxa_get_screen(pGC->pScreen);
/* fbValidateGC will do direct access to pixmaps if the tiling has
* changed.
* Preempt fbValidateGC by doing its work and masking the change out, so
* that we can do the Prepare/finish_access.
*/
+
+ /* If we are using GLAMOR, then the tile or stipple pixmap
+* may be pure GLAMOR pixmap, then we should let the glamor
+* to do the validation.
+*/
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ glamor_validate_gc(pGC, changes, pDrawable);
+ goto set_ops;
+ }
#ifdef FB_24_32BIT
if ((changes & GCTile) && fbGetRotatedPixmap(pGC)) {
(*pGC->pScreen->DestroyPixmap) (fbGetRotatedPixmap(pGC));
@@ -256,6 +267,7 @@ uxa_validate_gc(GCPtr pGC, unsigned long changes,
DrawablePtr pDrawable)
fbValidateGC(pGC, changes, pDrawable);
}
+set_ops:
pGC->ops = (GCOps *) & uxa_ops;
}
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] uxa/glamor: Remove extraneous flush.
From: Zhigang Gong
When glamor is enabled, a pixmap will not be accessed by the UXA's
accelerated function. Only un-accelerated funtion may access those
pixmap, and before each un-accelerated rendering, it calls
uxa_prepare_access which will do glFlush there.
Signed-off-by: Zhigang Gong
---
src/intel_glamor.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 2b56ae5..669b1b8 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -135,7 +135,6 @@ intel_glamor_finish_access(PixmapPtr pixmap, uxa_access_t
access)
break;
case UXA_GLAMOR_ACCESS_RW:
intel_glamor_need_flush(&pixmap->drawable);
- glamor_block_handler(pixmap->drawable.pScreen);
break;
default:
ErrorF("Invalid access mode %d\n", access);
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] uxa/glamor: Remove dead code.
From: Zhigang Gong
Signed-off-by: Zhigang Gong
---
src/intel_glamor.c | 13 -
src/intel_glamor.h |2 --
2 files changed, 0 insertions(+), 15 deletions(-)
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 8daa4b1..2b56ae5 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -192,19 +192,6 @@ intel_glamor_flush(intel_screen_private * intel)
}
Bool
-intel_glamor_create_screen_image(ScreenPtr screen, int handle, int stride)
-{
- ScrnInfoPtr scrn = xf86Screens[screen->myNum];
- intel_screen_private *intel;
-
- intel = intel_get_screen_private(scrn);
- if ((intel->uxa_flags & UXA_USE_GLAMOR) == 0)
- return TRUE;
-
- return glamor_egl_create_textured_screen(screen, handle, stride);
-}
-
-Bool
intel_glamor_close_screen(ScreenPtr screen)
{
ScrnInfoPtr scrn = xf86Screens[screen->myNum];
diff --git a/src/intel_glamor.h b/src/intel_glamor.h
index 1374588..3065132 100644
--- a/src/intel_glamor.h
+++ b/src/intel_glamor.h
@@ -35,7 +35,6 @@
Bool intel_glamor_pre_init(ScrnInfoPtr scrn);
Bool intel_glamor_init(ScreenPtr screen);
Bool intel_glamor_create_screen_resources(ScreenPtr screen);
-Bool intel_glamor_create_screen_image(ScreenPtr screen, int handle, int
stride);
Bool intel_glamor_close_screen(ScreenPtr screen);
void intel_glamor_free_screen(int scrnIndex, int flags);
@@ -51,7 +50,6 @@ PixmapPtr intel_glamor_create_pixmap(ScreenPtr screen, int w,
int h,
static inline Bool intel_glamor_pre_init(ScrnInfoPtr scrn) { return TRUE; }
static inline Bool intel_glamor_init(ScreenPtr screen) { return TRUE; }
static inline Bool intel_glamor_create_screen_resources(ScreenPtr screen) {
return TRUE; }
-static inline Bool intel_glamor_create_screen_image(ScreenPtr screen, int
handle, int stride) { return TRUE; }
static inline Bool intel_glamor_close_screen(ScreenPtr screen) { return TRUE; }
static inline void intel_glamor_free_screen(int scrnIndex, int flags) { }
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] uxa/glamor: performance tuning.
From: Zhigang Gong According to your advice, split the previous 1 patch to 4. And slightly changed the patch of create GC. The previous verison will override the GC ops to glamor's GC ops which is really not what I expected. If there are still some other unexpected side effects, please help to point out thanks. This patchset need to work with the latest glamor. I will move glamor to official repository this week and gave an initial version 0.1.0 and will bump the version number each time the API changed. Signed-off-by: Zhigang Gong - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] uxa/glamor: Prefer to create a pure glamor pixmap.
From: Zhigang Gong
Prefer to create a pure glamor pixmap rather than a BO and then
create a texture from that BO in glamor. As this will avoid some
glFlush requirements, it can get about 20-30% performance gain.
When glamor is enabled, route the uxa_create_gc to glamor_create_gc.
This commit also removed the glFlush at the intel_glamor_finish_access.
When glamor is enabled, a pixmap will not be accessed by the UXA's
accelerated function. Only un-accelerated funtion may access those
pixmap, and before each un-accelerated rendering, it calls
uxa_prepare_access which will do glFlush there.
Removed deprecated function: intel_glamor_create_screen_image
Signed-off-by: Zhigang Gong
---
src/intel_glamor.c | 14 --
src/intel_glamor.h |2 --
src/intel_uxa.c|6 ++
uxa/uxa-glamor.h |1 +
uxa/uxa.c |6 ++
5 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 8daa4b1..669b1b8 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -135,7 +135,6 @@ intel_glamor_finish_access(PixmapPtr pixmap, uxa_access_t
access)
break;
case UXA_GLAMOR_ACCESS_RW:
intel_glamor_need_flush(&pixmap->drawable);
- glamor_block_handler(pixmap->drawable.pScreen);
break;
default:
ErrorF("Invalid access mode %d\n", access);
@@ -192,19 +191,6 @@ intel_glamor_flush(intel_screen_private * intel)
}
Bool
-intel_glamor_create_screen_image(ScreenPtr screen, int handle, int stride)
-{
- ScrnInfoPtr scrn = xf86Screens[screen->myNum];
- intel_screen_private *intel;
-
- intel = intel_get_screen_private(scrn);
- if ((intel->uxa_flags & UXA_USE_GLAMOR) == 0)
- return TRUE;
-
- return glamor_egl_create_textured_screen(screen, handle, stride);
-}
-
-Bool
intel_glamor_close_screen(ScreenPtr screen)
{
ScrnInfoPtr scrn = xf86Screens[screen->myNum];
diff --git a/src/intel_glamor.h b/src/intel_glamor.h
index 1374588..3065132 100644
--- a/src/intel_glamor.h
+++ b/src/intel_glamor.h
@@ -35,7 +35,6 @@
Bool intel_glamor_pre_init(ScrnInfoPtr scrn);
Bool intel_glamor_init(ScreenPtr screen);
Bool intel_glamor_create_screen_resources(ScreenPtr screen);
-Bool intel_glamor_create_screen_image(ScreenPtr screen, int handle, int
stride);
Bool intel_glamor_close_screen(ScreenPtr screen);
void intel_glamor_free_screen(int scrnIndex, int flags);
@@ -51,7 +50,6 @@ PixmapPtr intel_glamor_create_pixmap(ScreenPtr screen, int w,
int h,
static inline Bool intel_glamor_pre_init(ScrnInfoPtr scrn) { return TRUE; }
static inline Bool intel_glamor_init(ScreenPtr screen) { return TRUE; }
static inline Bool intel_glamor_create_screen_resources(ScreenPtr screen) {
return TRUE; }
-static inline Bool intel_glamor_create_screen_image(ScreenPtr screen, int
handle, int stride) { return TRUE; }
static inline Bool intel_glamor_close_screen(ScreenPtr screen) { return TRUE; }
static inline void intel_glamor_free_screen(int scrnIndex, int flags) { }
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 9d74554..f04a2ef 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -1026,6 +1026,12 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
struct intel_pixmap *priv;
PixmapPtr pixmap, new_pixmap = NULL;
+ if (!(usage & INTEL_CREATE_PIXMAP_DRI2)) {
+ pixmap = intel_glamor_create_pixmap(screen, w, h, depth, usage);
+ if (pixmap)
+ return pixmap;
+ }
+
if (w > 32767 || h > 32767)
return NullPixmap;
diff --git a/uxa/uxa-glamor.h b/uxa/uxa-glamor.h
index 71a9c29..2b4b452 100644
--- a/uxa/uxa-glamor.h
+++ b/uxa/uxa-glamor.h
@@ -51,6 +51,7 @@
#define glamor_trapezoids_nf(...) FALSE
#define glamor_triangles_nf(...) FALSE
#define glamor_add_traps_nf(...) FALSE
+#define glamor_create_gc(...) FALSE
#endif
#endif /* UXA_GLAMOR_H */
diff --git a/uxa/uxa.c b/uxa/uxa.c
index 5b3d709..72d5ffd 100644
--- a/uxa/uxa.c
+++ b/uxa/uxa.c
@@ -38,6 +38,7 @@
#include
#include "dixfontstr.h"
#include "uxa.h"
+#include "uxa-glamor.h"
#if HAS_DEVPRIVATEKEYREC
DevPrivateKeyRec uxa_screen_index;
@@ -275,6 +276,11 @@ static GCFuncs uxaGCFuncs = {
*/
static int uxa_create_gc(GCPtr pGC)
{
+ uxa_screen_t *uxa_screen = uxa_get_screen(pGC->pScreen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR)
+ return glamor_create_gc(pGC);
+
if (!fbCreateGC(pGC))
return FALSE;
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] uxa/glamor: Fallback to new glamor pixmap if failed to create textured pixmap.
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Friday, December 16, 2011 5:21 PM
> To: zhigang.g...@linux.intel.com
> Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com; Zhigang
> Gong
> Subject: Re: [PATCH] uxa/glamor: Fallback to new glamor pixmap if failed
> to create textured pixmap.
>
> On Fri, 16 Dec 2011 15:39:55 +0800, zhigang.g...@linux.intel.com
> wrote:
> > +fallback_glamor:
> > + /* Create textured pixmap failed means glamor failed to
> > +* create a texture from current BO for some reasons. We turn
> > +* to create a new glamor pixmap and clean up current one.
> > +* One thing need to be noted, this new pixmap doesn't
> > +* has a priv and bo attached to it. It's glamor's responsbility
> > +* to take care it.
> > +*/
> This then fails intel_uxa_is_offscreen() and we can no longer fallback to
> swrast correctly as uxa_prepare_access() becomes a no-op.
Right. IMO, this is not a big problem here, as in glamor this type
of pixmap will be marked as texture only pixmap and will never
fallback to DDX layer for any rendering operation.
>
> > + if (usage & INTEL_CREATE_PIXMAP_DRI2) {
> > + xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> > + "Failed to create textured DRI2 pixmap.");
> > + return pixmap;
> At which point we really do need a better means for integrating glamor
> and UXA ops...
Actually, I haven't thought out a good idea for this case , say how to
handle such a BO
only pixmap in glamor. For almost all the other scenarios, we can easily
avoid
BO only pixmap by using a texture only pixmap or an in-memory pixmap. This
case is an exception.
And if I mesa/gbm can support all DRI format, then this case will also be
eliminated, as
It should never fail at glamor side. And if it failed we can just abort, as
that may indicates
a serious low level problem.
If we have to live with a BO only pixmap in glamor environment, one possible
fallback
method is to create another compatible BO for the same DRI2 pixmap. Then at
prepare
access glamor, we can copy the real BO's content to the compatible BO, and
at
finish access glamor, we can copy back the compatible BO's content to the
real DRI2
buffer's BO.
Does this way work?
But I really want the mesa/gbm to support all DRI2 formats and then
>
> > + }
> > + new_pixmap = intel_glamor_create_pixmap(screen, w, h,
> > + depth, usage);
> > dri_bo_unreference(priv->bo);
> > fallback_priv:
> > free(priv);
> > fallback_pixmap:
> > fbDestroyPixmap(pixmap);
> > - return fbCreatePixmap(screen, w, h, depth, usage);
> > + if (new_pixmap)
> > + return new_pixmap;
> > + else
> > + return fbCreatePixmap(screen, w, h, depth, usage);
> > }
>
> Otherwise, it does look to be a step in the right direction.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] One regression in UXA
Forgot to mention that to access that link need to be in Intel intranet environment. From: intel-gfx-bounces+zhigang.gong=linux.intel@lists.freedesktop.org [mailto:intel-gfx-bounces+zhigang.gong=linux.intel@lists.freedesktop.org ] On Behalf Of Zhigang Gong Sent: Friday, December 16, 2011 4:09 PM To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] One regression in UXA Hi Chris, I tested a browser case with latest UXA code and experienced X crashed. I bisected it and found that it was introduced by commit 5d5b2b8ee203ae2274fc7d13ed38d2945facca9e. The back trace of the crash is as below: Backtrace: 0: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (xorg_backtrace+0x3b) [0x81b5fdb] 1: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (mieqEnqueue+0x15c) [0x8195a0c] 2: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (0x8048000+0x40782) [0x8088782] 3: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (xf86PostMotionEventM+0xf1) [0x80c5671] 4: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/input/evdev_drv.so (0xb7623000+0x2fdf) [0xb7625fdf] 5: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/input/evdev_drv.so (0xb7623000+0x4229) [0xb7627229] 6: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (0x8048000+0x67fcf) [0x80affcf] 7: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (0x8048000+0x8e984) [0x80d6984] 8: (vdso) (__kernel_sigreturn+0x0) [0xb7fff400] 9: /home/gongzg/gfx-dev-test/build/lib/libdrm_intel.so.1 (drm_intel_gem_bo_map_gtt+0x83) [0xb7e9cee3] 10: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/drivers/intel_drv.so (0xb7ea1000+0xd6cb) [0xb7eae6cb] -- intel_uxa_pixmap_put_image 11: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/drivers/intel_drv.so (0xb7ea1000+0xf42c) [0xb7eb042c] -- intel_uxa_put_image 12: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/drivers/intel_drv.so (0xb7ea1000+0x37f8e) [0xb7ed8f8e] -- uxa_trapezoids It's a one line patch. And there is not too much thing to do with UXA code, I guess It may be a libdrm related problem. It's can be simply reproduced in my machine, Use Mozilla 3.6.10 to open the following URL: http://pnp.sh.intel.com/WRTBench/Workload_3.3_rev0.13/iterations_rev11_new.h tml It will iterate many pages and finally give a report. The crash may occur after a few seconds of running. Uncomment that one line patch can avoid this crash. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] One regression in UXA
Hi Chris, I tested a browser case with latest UXA code and experienced X crashed. I bisected it and found that it was introduced by commit 5d5b2b8ee203ae2274fc7d13ed38d2945facca9e. The back trace of the crash is as below: Backtrace: 0: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (xorg_backtrace+0x3b) [0x81b5fdb] 1: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (mieqEnqueue+0x15c) [0x8195a0c] 2: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (0x8048000+0x40782) [0x8088782] 3: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (xf86PostMotionEventM+0xf1) [0x80c5671] 4: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/input/evdev_drv.so (0xb7623000+0x2fdf) [0xb7625fdf] 5: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/input/evdev_drv.so (0xb7623000+0x4229) [0xb7627229] 6: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (0x8048000+0x67fcf) [0x80affcf] 7: /home/gongzg/gfx-dev-test/src/xserver/hw/xfree86/Xorg (0x8048000+0x8e984) [0x80d6984] 8: (vdso) (__kernel_sigreturn+0x0) [0xb7fff400] 9: /home/gongzg/gfx-dev-test/build/lib/libdrm_intel.so.1 (drm_intel_gem_bo_map_gtt+0x83) [0xb7e9cee3] 10: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/drivers/intel_drv.so (0xb7ea1000+0xd6cb) [0xb7eae6cb] -- intel_uxa_pixmap_put_image 11: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/drivers/intel_drv.so (0xb7ea1000+0xf42c) [0xb7eb042c] -- intel_uxa_put_image 12: /home/gongzg/gfx-dev-test/build/lib/xorg/modules/drivers/intel_drv.so (0xb7ea1000+0x37f8e) [0xb7ed8f8e] -- uxa_trapezoids It's a one line patch. And there is not too much thing to do with UXA code, I guess It may be a libdrm related problem. It's can be simply reproduced in my machine, Use Mozilla 3.6.10 to open the following URL: http://pnp.sh.intel.com/WRTBench/Workload_3.3_rev0.13/iterations_rev11_new.h tml It will iterate many pages and finally give a report. The crash may occur after a few seconds of running. Uncomment that one line patch can avoid this crash. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] uxa/glamor: Fallback to new glamor pixmap if failed to create textured pixmap.
From: Zhigang Gong
If we failed to create textured pixmap from BO's handle, we
turn to create a new glamor pixmap by call glamor_create_pixmap
rather than fallback to in-memory pixmap. Have to introduce
a new wrapper function intel_glamor_create_pixmap.
Signed-off-by: Zhigang Gong
---
src/intel_glamor.c |7 +++
src/intel_glamor.h |5 +
src/intel_uxa.c| 39 ---
3 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 0cf8ed7..e63b9f6 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -78,6 +78,13 @@ intel_glamor_pre_init(ScrnInfoPtr scrn)
return TRUE;
}
+PixmapPtr
+intel_glamor_create_pixmap(ScreenPtr screen, int w, int h,
+ int depth, unsigned int usage)
+{
+ return glamor_create_pixmap(screen, w, h, depth, usage);
+}
+
Bool
intel_glamor_create_textured_pixmap(PixmapPtr pixmap)
{
diff --git a/src/intel_glamor.h b/src/intel_glamor.h
index 1ba17c0..1374588 100644
--- a/src/intel_glamor.h
+++ b/src/intel_glamor.h
@@ -43,6 +43,8 @@ void intel_glamor_flush(intel_screen_private * intel);
Bool intel_glamor_create_textured_pixmap(PixmapPtr pixmap);
void intel_glamor_destroy_pixmap(PixmapPtr pixmap);
+PixmapPtr intel_glamor_create_pixmap(ScreenPtr screen, int w, int h,
+int depth, unsigned int usage);
#else
@@ -58,6 +60,9 @@ static inline void intel_glamor_flush(intel_screen_private *
intel) { }
static inline Bool intel_glamor_create_textured_pixmap(PixmapPtr pixmap) {
return TRUE; }
static inline void intel_glamor_destroy_pixmap(PixmapPtr pixmap) { }
+static inline PixmapPtr intel_glamor_create_pixmap(ScreenPtr screen, int w,
int h,
+ int depth, unsigned int
usage) { return NULL; }
+
#endif
#endif /* INTEL_GLAMOR_H */
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 292642e..de36f67 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -1024,7 +1024,7 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
ScrnInfoPtr scrn = xf86Screens[screen->myNum];
intel_screen_private *intel = intel_get_screen_private(scrn);
struct intel_pixmap *priv;
- PixmapPtr pixmap;
+ PixmapPtr pixmap, new_pixmap = NULL;
if (w > 32767 || h > 32767)
return NullPixmap;
@@ -1110,9 +1110,8 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
screen->ModifyPixmapHeader(pixmap, w, h, 0, 0,
stride, NULL);
- if
(!intel_glamor_create_textured_pixmap(pixmap))
- goto fallback_bo;
-
+ if
(!intel_glamor_create_textured_pixmap(pixmap))
+ goto fallback_glamor;
return pixmap;
}
}
@@ -1146,26 +1145,36 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
screen->ModifyPixmapHeader(pixmap, w, h, 0, 0, stride, NULL);
- /* Create textured pixmap failed means glamor fail to create
-* a texture from the BO for some reasons, and then glamor
-* create a new texture attached to the pixmap, and all the
-* consequent rendering operations on this pixmap will never
-* fallback to UXA path, so we don't need to hold the useless
-* BO if it is the case.
-*/
- if (!intel_glamor_create_textured_pixmap(pixmap))
- goto fallback_bo;
+ if (!intel_glamor_create_textured_pixmap(pixmap))
+ goto fallback_glamor;
}
return pixmap;
-fallback_bo:
+fallback_glamor:
+ /* Create textured pixmap failed means glamor failed to
+* create a texture from current BO for some reasons. We turn
+* to create a new glamor pixmap and clean up current one.
+* One thing need to be noted, this new pixmap doesn't
+* has a priv and bo attached to it. It's glamor's responsbility
+* to take care it.
+*/
+ if (usage & INTEL_CREATE_PIXMAP_DRI2) {
+ xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+ "Failed to create textured DRI2 pixmap.");
+ return pixmap;
+ }
+ new_pixmap = intel_glamor_create_pixmap(screen, w, h,
+ depth, usage);
dri_bo_unreference(priv->bo);
fallback_priv:
free(priv);
fallback_pixmap:
fbDestroyPixmap(pixmap);
- return fbCreatePixmap(screen, w, h, depth, usage);
+ if (new_pixmap)
+ return new_pixmap;
+ else
+ return fbCreatePixmap(screen, w, h, depth,
Re: [Intel-gfx] [PATCH] uxa/glamor: Enable the rest glamor rendering functions.
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Wednesday, December 14, 2011 7:12 PM
> To: Zhigang Gong
> Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com
> Subject: RE: [PATCH] uxa/glamor: Enable the rest glamor rendering
> functions.
>
> On Wed, 14 Dec 2011 12:08:30 +0800, "Zhigang Gong"
> wrote:
> > > -Original Message-
> > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > > Sent: Wednesday, December 14, 2011 2:45 AM
> > > To: zhigang.g...@linux.intel.com
> > > Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com;
> > > zhigang.g...@linux.intel.com
> > > Subject: Re: [PATCH] uxa/glamor: Enable the rest glamor rendering
> > > functions.
> > >
> > > On Tue, 13 Dec 2011 22:31:41 +0800, zhigang.g...@linux.intel.com
> > > wrote:
> > > > From: Zhigang Gong
> > > >
> > > > This commit enable all the rest glamor rendering functions.
> > > > Tested with latest glamor master branch, can pass rendercheck.
> > >
> > > Hmm, it exposes an issue with keeping a bo cache independent of
> mesa
> > > and trying to feed it our own handles:
> > >
> > > Region for name 6 already exists but is not compatible
> > >
> > > The w/a for this would be:
> > >
> > > diff --git a/src/intel_glamor.c b/src/intel_glamor.c index
> > 0cf8ed7..2757fd6
> > > 100644
> > > --- a/src/intel_glamor.c
> > > +++ b/src/intel_glamor.c
> > > @@ -91,6 +91,7 @@
> intel_glamor_create_textured_pixmap(PixmapPtr
> > > pixmap)
> > > priv = intel_get_pixmap_private(pixmap);
> > > if (glamor_egl_create_textured_pixmap(pixmap,
> > > priv->bo->handle,
> > > priv->stride)) {
> > > + drm_intel_bo_disable_reuse(priv->bo);
> > > priv->pinned = 1;
> > > return TRUE;
> > > } else
> > >
> > > but that gives up all pretense of maintaining a bo cache.
> >
> > Yes, I think this impacts the performance. Actually, I noticed this
> > problem and I spent some time to track the root cause. If everything
> > is ok, this error should not be triggered. Although the same BO maybe
> > reused to create a new pixmap, the previous pixmap which own this BO
> > should be already destroyed. And the previous image created with the
> > previous pixmap should be destroyed either.
>
> Certainly it looks like glamor is taking all necessary steps to decouple
the
> bo from the textures and renderbuffer upon pixmap finish. There is one
> other potential race here in that the ddx will mark the bo as purgeable as
> soon as it releases it back to the cache, but it may not yet have been
> submitted by mesa in its execbuffer. The kernel may choose to free the
> memory associated with the bo before that happens, and may rightfully
> complain the userspace is doing something silly.
Right, we do have this race if the kernel free the BO's memory prior to
The mesa submit its execbuffer. Hmm. But I think that may not be a real
problem, as once we call intel_set_pixmap_bo(pixmap, NULL) to unlink
the bo from the pixmap, the BO will not be released at DRM layer
immediately,
instead, it will be put on a in_flight list. And intel_batch_submit will
empty the
list, considering after switching to glamor, each pixmap's batch buffer
should
be empty, then the driver will only call intel_batch_submit at
intel_flush_rendering
which is called from intel_uxa_block_handler and is after the
intel_glamor_flush.
At intel_glamor_flush, it will do a glFlush, my understanding is glFlush
should make sure the execbuffer was submitted to GPU. But I'm not very sure.
Can you confirm that? Thanks.
>
> > And then, when we create a new pixmap/image with this BO, MESA
> should
> > not find any exist image/region for this BO. But it does happen. I
> > tracked further into mesa internal and found that the previous image
> > was not destroyed when we call eglDestroyImageKHR, as its reference
> > count is decreased to zero. It's weird for me. Further tracking shows
> > that the root cause is when I use the texture(bind to the image) as a
> > shader's source texture, and call glDrawArrays to perform the
> > rendering, the texture's reference count will be increased by 1 before
> > return from glDrawArrays. And I failed to find any API to decrease it.
> > Then this texture can't be freed when destroy that texture and thus
> > the image's referen
Re: [Intel-gfx] [PATCH] uxa/glamor: Enable the rest glamor rendering functions.
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Wednesday, December 14, 2011 2:45 AM
> To: zhigang.g...@linux.intel.com
> Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com;
> zhigang.g...@linux.intel.com
> Subject: Re: [PATCH] uxa/glamor: Enable the rest glamor rendering
> functions.
>
> On Tue, 13 Dec 2011 22:31:41 +0800, zhigang.g...@linux.intel.com
> wrote:
> > From: Zhigang Gong
> >
> > This commit enable all the rest glamor rendering functions.
> > Tested with latest glamor master branch, can pass rendercheck.
>
> Hmm, it exposes an issue with keeping a bo cache independent of mesa
> and trying to feed it our own handles:
>
> Region for name 6 already exists but is not compatible
>
> The w/a for this would be:
>
> diff --git a/src/intel_glamor.c b/src/intel_glamor.c index
0cf8ed7..2757fd6
> 100644
> --- a/src/intel_glamor.c
> +++ b/src/intel_glamor.c
> @@ -91,6 +91,7 @@ intel_glamor_create_textured_pixmap(PixmapPtr
> pixmap)
> priv = intel_get_pixmap_private(pixmap);
> if (glamor_egl_create_textured_pixmap(pixmap,
> priv->bo->handle,
> priv->stride)) {
> + drm_intel_bo_disable_reuse(priv->bo);
> priv->pinned = 1;
> return TRUE;
> } else
>
> but that gives up all pretense of maintaining a bo cache.
Yes, I think this impacts the performance. Actually, I noticed this problem
and I
spent some time to track the root cause. If everything is ok, this error
should
not be triggered. Although the same BO maybe reused to create a new pixmap,
the previous pixmap which own this BO should be already destroyed. And the
previous image created with the previous pixmap should be destroyed either.
And then, when we create a new pixmap/image with this BO, MESA should not
find any exist image/region for this BO. But it does happen. I tracked
further into
mesa internal and found that the previous image was not destroyed when we
call eglDestroyImageKHR, as its reference count is decreased to zero. It's
weird
for me. Further tracking shows that the root cause is when I use the
texture(bind to
the image) as a shader's source texture, and call glDrawArrays to perform
the
rendering, the texture's reference count will be increased by 1 before
return
from glDrawArrays. And I failed to find any API to decrease it. Then this
texture
can't be freed when destroy that texture and thus the image's reference
count
will also remain 1 and can't be freed either.
The attached is a simple case to show this bug. It was modified from the
eglkms.c
in mesa-demos.
I did send this issue to mesa-dev. Don't have a solution or explanation so
far. Any
idea?
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
eglkms_mod.c
Description: Binary data
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] uxa/glamor: Enable the rest glamor rendering functions.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Wednesday, December 14, 2011 2:20 AM > To: zhigang.g...@linux.intel.com > Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com; > zhigang.g...@linux.intel.com > Subject: Re: [PATCH] uxa/glamor: Enable the rest glamor rendering > functions. > > On Tue, 13 Dec 2011 22:31:41 +0800, zhigang.g...@linux.intel.com > wrote: > > From: Zhigang Gong > > > > This commit enable all the rest glamor rendering functions. > > Tested with latest glamor master branch, can pass rendercheck. > > > > One thing need to be pointed out is the picture's handling. > > Pictures support many different color formats, but glamor's texture > > only support a few color formats. And the most common scenario is that > > we create a pixmap with a color depth and then attach it to a picture > > which has a specific color format with the same color depth. But there > > is no way to change a texture's internal format after the texture was > > allocated. > > If you do that, the OpenGL will allocate a new texture. And then the > > glamor side and UXA side will be inconsitence. So for all the picture > > related operations, we can't fallback to UXA path directly, even it is > > rather a strainth forward operation. So for the get_image, Addtraps.., > > we have to add wrappers function for them to jump into glamor firstly. > > Can we create multiple textures referencing the same bo but with > different formats? AFAIK, it's impossible to match all possible picture formats to a OpenGL internal format. We have to have a new texture attached to glamor for incompatible format. The old texture is created from DDX's BO and has incorrect internal format. IMO, we can't make any use of this wrong texture within glamor, so I just don't create it and return a false to DDX layer to notify the DDX to unlink the BO. All the consequent rendering operation on this pixmap will be handled within glamor scope and target to the new texture with correct format. > Or are we going to run afoul of the coherency model > with GL? My understanding is, if the picture's format is incompatible with OpenGL's internal format. --Zhigang > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] uxa/glamor: Enable the rest glamor rendering functions.
From: Zhigang Gong
This commit enable all the rest glamor rendering functions.
Tested with latest glamor master branch, can pass rendercheck.
One thing need to be pointed out is the picture's handling.
Pictures support many different color formats, but glamor's
texture only support a few color formats. And the most common
scenario is that we create a pixmap with a color depth and
then attach it to a picture which has a specific color format
with the same color depth. But there is no way to change a
texture's internal format after the texture was allocated.
If you do that, the OpenGL will allocate a new texture. And
then the glamor side and UXA side will be inconsitence. So
for all the picture related operations, we can't fallback to
UXA path directly, even it is rather a strainth forward
operation. So for the get_image, Addtraps.., we have to add
wrappers function for them to jump into glamor firstly.
Signed-off-by: Zhigang Gong
---
src/intel_uxa.c | 14 +++-
uxa/uxa-accel.c | 88 -
uxa/uxa-glamor.h | 16 -
uxa/uxa-glyphs.c | 21
uxa/uxa-priv.h |8 +
uxa/uxa-render.c | 90 ++
uxa/uxa.c|4 +-
7 files changed, 234 insertions(+), 7 deletions(-)
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index e4a5270..a79affa 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -1108,6 +1108,9 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
list_del(&priv->in_flight);
screen->ModifyPixmapHeader(pixmap, w, h, 0, 0,
stride, NULL);
intel_set_pixmap_private(pixmap, priv);
+
+ if
(!intel_glamor_create_textured_pixmap(pixmap))
+ intel_set_pixmap_bo(pixmap, NULL);
return pixmap;
}
}
@@ -1145,8 +1148,15 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
list_init(&priv->batch);
list_init(&priv->flush);
intel_set_pixmap_private(pixmap, priv);
-
- intel_glamor_create_textured_pixmap(pixmap);
+ /* Create textured pixmap failed means glamor fail to create
+* a texture from the BO for some reasons, and then glamor
+* create a new texture attached to the pixmap, and all the
+* consequent rendering operations on this pixmap will never
+* fallback to UXA path, so we don't need to hold the useless
+* BO if it is the case.
+*/
+ if (!intel_glamor_create_textured_pixmap(pixmap))
+ intel_set_pixmap_bo(pixmap, NULL);
}
return pixmap;
diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index e4afd13..05c64f6 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -207,8 +207,23 @@ static void
uxa_put_image(DrawablePtr pDrawable, GCPtr pGC, int depth, int x, int y,
int w, int h, int leftPad, int format, char *bits)
{
+ uxa_screen_t *uxa_screen = uxa_get_screen(pDrawable->pScreen);
+
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ uxa_prepare_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ if (glamor_put_image_nf(pDrawable,
+ pGC, depth, x, y, w, h,
+ leftPad, format, bits)) {
+ uxa_finish_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ return;
+ }
+ uxa_finish_access(pDrawable, UXA_GLAMOR_ACCESS_RO);
+ goto fallback;
+ }
+
if (!uxa_do_put_image(pDrawable, pGC, depth, x, y, w, h, format, bits,
PixmapBytePad(w, pDrawable->depth)))
+fallback:
uxa_check_put_image(pDrawable, pGC, depth, x, y, w, h, leftPad,
format, bits);
}
@@ -352,6 +367,22 @@ uxa_copy_n_to_n(DrawablePtr pSrcDrawable,
int dst_off_x, dst_off_y;
PixmapPtr pSrcPixmap, pDstPixmap;
+ if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+ uxa_prepare_access(pSrcDrawable, UXA_GLAMOR_ACCESS_RO);
+ uxa_prepare_access(pDstDrawable, UXA_GLAMOR_ACCESS_RW);
+ if (glamor_copy_n_to_n_nf(pSrcDrawable, pDstDrawable,
+ pGC, pbox, nbox, dx, dy,
+ reverse, upsidedown, bitplane,
+ closure)) {
+ uxa_finish_access(pDstDrawable, UXA_GLAMOR_ACCESS_RW);
+ uxa_finish_access(pSrcDrawable, UXA_GLAMOR_ACCESS_RO);
+
Re: [Intel-gfx] [PATCH 0/2] Introduce Glamor to UXA framework.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Thursday, November 17, 2011 6:57 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Subject: RE: [PATCH 0/2] Introduce Glamor to UXA framework. > > On Thu, 17 Nov 2011 18:49:49 +0800, "Zhigang Gong" > wrote: > > > -Original Message- > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > Sent: Thursday, November 17, 2011 5:20 PM > > > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > > > Subject: RE: [PATCH 0/2] Introduce Glamor to UXA framework. > > > > > > On Thu, 17 Nov 2011 10:55:39 +0800, "Zhigang Gong" > > > wrote: > > > > Now glamor was extracted from the xorg and be a separate library. > > > > Please checkout glamor at : > > > git://people.freedesktop.org/~gongzg/glamor. > > > > This library implements two modules glamor and glamor_egl which > > > > are required for intel video driver to enable glamor. > > > > > > Hmm, I did install the glamor library. A > > /usr/lib/xorg/modules/libglamor.so > > > is installed but the ddx is looking for a "glamor_egl" module. > > > > > > Ah, looks to be due to the > > > checking for EGL... no > > > checking for EGL... yes > > > in glamor. How very confusing. ;-) > > I think I know the reason, there is a typo error in glamor's > > configure.ac which doesn't Check the required packages correctly. > > Please pull the latest glamor master. > > One of the > > two above checks should be checking for GBM. I guess your mesa hasn't > > been built with gbm enabled. Then glamor will not build glamor_egl > > which depends on that. > > It was actually mesa insisting on only building egl_platform=drm and the > pre-requisite egl_dri2 if and only if libxcb-dri2 was installed. Seems a bit > strange for something that is supposed to be used on an X-less system. > > > Another thing need to pay attention is that when you build xserver, > > you need to disable glx. > > Rebuild the xserver... Oh well, that rules out testing by simply replacing > distro drivers. One of the todo items is to fully support glx including direct and indirect . The major problem is that Xserver's glx code has the same glapi symbols which conflicts with standard mesa's glapi's implementations. It's easy to rename the xserver side glapi's symbol to other names to avoid this conflicts, but that will break indirect GLX in xserver side. I still don't have a perfect solution to solve this problem. Do you or anyone in this list have any suggestion to solve this confliction? > I'm having a lot of fun with building glamor as it is missing or > including headers in the wrong order and deciding that XID is > 8 bytes when the server was compiled with a 4 byte XID. Fun. Will discuss these problem with you through IRC. > And the > usual problems with incomplete framebuffers, but hopefully that is just the > glx confusion. That must be caused by glapi symbol confliction. Thanks. - Zhigang > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/2] Introduce Glamor to UXA framework.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Thursday, November 17, 2011 5:20 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Subject: RE: [PATCH 0/2] Introduce Glamor to UXA framework. > > On Thu, 17 Nov 2011 10:55:39 +0800, "Zhigang Gong" > wrote: > > Now glamor was extracted from the xorg and be a separate library. > > Please checkout glamor at : > git://people.freedesktop.org/~gongzg/glamor. > > This library implements two modules glamor and glamor_egl which are > > required for intel video driver to enable glamor. > > Hmm, I did install the glamor library. A /usr/lib/xorg/modules/libglamor.so > is installed but the ddx is looking for a "glamor_egl" module. > > Ah, looks to be due to the > checking for EGL... no > checking for EGL... yes > in glamor. How very confusing. ;-) I think I know the reason, there is a typo error in glamor's configure.ac which doesn't Check the required packages correctly. Please pull the latest glamor master. One of the two above checks should be checking for GBM. I guess your mesa hasn't been built with gbm enabled. Then glamor will not build glamor_egl which depends on that. Another thing need to pay attention is that when you build xserver, you need to disable glx. Sorry for the confusing. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/2] Introduce Glamor to UXA framework.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Thursday, November 17, 2011 9:14 AM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Cc: zhigang.g...@linux.intel.com > Subject: Re: [PATCH 0/2] Introduce Glamor to UXA framework. > > On Wed, 16 Nov 2011 15:04:35 +0800, Zhigang Gong > wrote: > > This patchset initially enable glamor with UXA. And two functions > > ,fill_spans and poly_fill_rects, go to the glamor path. I tested it > > with render check, and it works fine. > > I split your patches slightly differently and pushed them. I just checked out the master branch, and have a simple test, everything seems ok. Thanks. > I could only verify > that it didn't impact UXA without the glamor_egl module available. Do you > have a patch for testing? Now glamor was extracted from the xorg and be a separate library. Please checkout glamor at : git://people.freedesktop.org/~gongzg/glamor. This library implements two modules glamor and glamor_egl which are required for intel video driver to enable glamor. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] glamor: Turn on glamor with a few glamor functions enabled.
Integrate glamor acceleration into UXA framework. Add
necessary flushing at the following points:
1. Flush UXA batch buffer before call into glamor.
2. Flush GL operations after return from a glamor function.
3. The point we need to flush UXA batch buffer, we also
need to flush GL operations, for example, in
intel_flush_callback and couple of places in intel_display.c.
This commit only enables two glamor functions for
fill_spans and poly_fill_rects. Will continue to enable the
reset glamor functions soon.
Signed-off-by: Zhigang Gong
---
src/Makefile.am |6 +--
src/intel.h |1 +
src/intel_display.c |5 ++
src/intel_driver.c | 17 +++-
src/intel_glamor.c | 111 ++-
src/intel_glamor.h |2 +-
src/intel_uxa.c | 22 +-
uxa/uxa-accel.c | 38 +++--
uxa/uxa-priv.h |4 +-
uxa/uxa-render.c| 16
uxa/uxa-unaccel.c | 58 +-
uxa/uxa.c | 18
uxa/uxa.h | 13 +-
13 files changed, 236 insertions(+), 75 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 1a29390..12ff6ee 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,10 +40,6 @@ SUBDIRS += sna
intel_drv_la_LIBADD += sna/libsna.la
endif
-if GLAMOR
-GLAMOR_SOURCE = intel_glamor.c
-endif
-
NULL:=#
intel_drv_la_SOURCES = \
@@ -74,7 +70,7 @@ intel_drv_la_SOURCES = \
i965_3d.c \
i965_video.c \
i965_render.c \
-$(GLAMOR_SOURCE) \
+intel_glamor.c \
$(NULL)
if DRI
diff --git a/src/intel.h b/src/intel.h
index 3b3f87d..b24aa02 100644
--- a/src/intel.h
+++ b/src/intel.h
@@ -318,6 +318,7 @@ typedef struct intel_screen_private {
void (*batch_commit_notify) (struct intel_screen_private *intel);
uxa_driver_t *uxa_driver;
+ int uxa_flags;
Bool need_sync;
int accel_pixmap_offset_alignment;
int accel_max_x;
diff --git a/src/intel_display.c b/src/intel_display.c
index 84c7c08..f6ffa30 100644
--- a/src/intel_display.c
+++ b/src/intel_display.c
@@ -43,6 +43,8 @@
#include "xf86drmMode.h"
#include "X11/Xatom.h"
+#include "intel_glamor.h"
+
struct intel_mode {
int fd;
uint32_t fb_id;
@@ -451,6 +453,7 @@ intel_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr
mode,
crtc->y = y;
crtc->rotation = rotation;
+ intel_glamor_flush(intel);
intel_batch_submit(crtc->scrn);
mode_to_kmode(crtc->scrn, &intel_crtc->kmode, mode);
@@ -1368,6 +1371,7 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int
height)
if (scrn->virtualX == width && scrn->virtualY == height)
return TRUE;
+ intel_glamor_flush(intel);
intel_batch_submit(scrn);
old_width = scrn->virtualX;
@@ -1457,6 +1461,7 @@ intel_do_pageflip(intel_screen_private *intel,
new_front->handle, &mode->fb_id))
goto error_out;
+ intel_glamor_flush(intel);
intel_batch_submit(scrn);
/*
diff --git a/src/intel_driver.c b/src/intel_driver.c
index 24696da..ddeb40f 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -75,6 +75,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "i915_drm.h"
#include
+#include "intel_glamor.h"
+
/* *INDENT-OFF* */
/*
* Note: "ColorKey" is provided for compatibility with the i810 driver.
@@ -712,6 +714,13 @@ static Bool I830PreInit(ScrnInfoPtr scrn, int flags)
return FALSE;
}
+ if (!intel_glamor_pre_init(scrn)) {
+ PreInitCleanup(scrn);
+ xf86DrvMsg(scrn->scrnIndex, X_ERROR,
+ "Failed to pre init glamor display.\n");
+ return FALSE;
+ }
+
/* Load the dri2 module if requested. */
if (intel->directRenderingType != DRI_DISABLED)
xf86LoadSubModule(scrn, "dri2");
@@ -813,8 +822,10 @@ intel_flush_callback(CallbackListPtr *list,
pointer user_data, pointer call_data)
{
ScrnInfoPtr scrn = user_data;
- if (scrn->vtSema)
+ if (scrn->vtSema) {
intel_batch_submit(scrn);
+ intel_glamor_flush(intel_get_screen_private(scrn));
+ }
}
#if HAVE_UDEV
@@ -1110,6 +1121,8 @@ static void I830FreeScreen(int scrnIndex, int flags)
ScrnInfoPtr scrn = xf86Screens[scrnIndex];
intel_screen_private *intel = intel_get_screen_private(scrn);
+ intel_glamor_free_screen(scrnIndex, flags);
+
if (intel) {
intel_mode_fini(intel);
intel_close_drm_master(intel);
@@ -1189,6 +1202,8 @@ static Bool I830CloseScreen(int scrnIndex, ScreenPtr
screen)
DeleteCallback(&FlushCallback, intel_flush_callback, scrn);
+
[Intel-gfx] [PATCH 1/2] glamor: Initial commit to introduce glamor acceleration.
Added one configuration option --enable-glamor to control
whether use glamor. Added one new file intel_glamor.c to
wrap glamor egl API for intel driver's usage.
This commit doesn't really change the driver's control path.
It just adds necessary files for glamor and change some
configuration.
Reviewed-by: Eugeni Dodonov
Signed-off-by: Zhigang Gong
---
configure.ac | 17 +++
src/Makefile.am|5 ++
src/intel_glamor.c | 130
src/intel_glamor.h | 45 ++
4 files changed, 197 insertions(+), 0 deletions(-)
create mode 100644 src/intel_glamor.c
create mode 100644 src/intel_glamor.h
diff --git a/configure.ac b/configure.ac
index 831f6b3..d990702 100644
--- a/configure.ac
+++ b/configure.ac
@@ -126,6 +126,15 @@ AC_ARG_ENABLE(sna,
[SNA="$enableval"],
[SNA=no])
AM_CONDITIONAL(SNA, test x$SNA != xno)
+
+AC_ARG_ENABLE(glamor,
+ AS_HELP_STRING([--enable-glamor],
+[Enable glamor, a new GL-based acceleration
[default=no]]),
+ [GLAMOR="$enableval"],
+ [GLAMOR=no])
+
+AM_CONDITIONAL(GLAMOR, test x$GLAMOR != xno)
+
AC_MSG_CHECKING([whether to include SNA support])
required_xorg_xserver_version=1.6
@@ -137,6 +146,14 @@ if test "x$SNA" != "xno"; then
fi
AC_MSG_RESULT([$SNA])
+if test "x$GLAMOR" != "xno"; then
+ PKG_CHECK_MODULES(LIBGLAMOR, [glamor])
+ PKG_CHECK_MODULES(LIBGLAMOR_EGL, [glamor-egl])
+ AC_DEFINE(GLAMOR, 1, [Enable glamor acceleration])
+fi
+
+AC_MSG_CHECKING([whether to include GLAMOR support])
+AC_MSG_RESULT([$GLAMOR])
AC_ARG_ENABLE(vmap,
AS_HELP_STRING([--enable-vmap],
diff --git a/src/Makefile.am b/src/Makefile.am
index cd1bb36..1a29390 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,6 +40,10 @@ SUBDIRS += sna
intel_drv_la_LIBADD += sna/libsna.la
endif
+if GLAMOR
+GLAMOR_SOURCE = intel_glamor.c
+endif
+
NULL:=#
intel_drv_la_SOURCES = \
@@ -70,6 +74,7 @@ intel_drv_la_SOURCES = \
i965_3d.c \
i965_video.c \
i965_render.c \
+$(GLAMOR_SOURCE) \
$(NULL)
if DRI
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
new file mode 100644
index 000..cadfc71
--- /dev/null
+++ b/src/intel_glamor.c
@@ -0,0 +1,130 @@
+/*
+ * Copyright © 2011 Intel Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including
+ * the next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *Zhigang Gong
+ *
+ */
+
+#ifdef HAVE_DIX_CONFIG_H
+#include
+#endif
+#include
+#include
+#include
+#include
+#include
+
+#define GLAMOR_FOR_XORG 1
+
+#include
+#include "intel.h"
+#include "intel_glamor.h"
+
+Bool
+intel_glamor_create_screen_resources(ScreenPtr screen)
+{
+ ScrnInfoPtr scrn = xf86Screens[screen->myNum];
+ intel_screen_private *intel = intel_get_screen_private(scrn);
+
+ if (!glamor_glyphs_init(screen))
+ return FALSE;
+ if (!glamor_egl_create_textured_screen(screen,
+ intel->front_buffer->handle,
+ intel->front_pitch))
+ return FALSE;
+ return TRUE;
+}
+
+Bool
+intel_glamor_pre_init(ScrnInfoPtr scrn)
+{
+ intel_screen_private *intel;
+ intel = intel_get_screen_private(scrn);
+ return glamor_egl_init(scrn, intel->drmSubFD);
+}
+
+Bool
+intel_glamor_create_textured_pixmap(PixmapPtr pixmap)
+{
+ struct intel_pixmap *priv;
+ priv = intel_get_pixmap_private(pixmap);
+ return glamor_egl_create_textured_pixmap(pixmap, priv->bo->handle,
+priv->stride);
+}
+
+void
+intel_glamor_destroy_pixmap(PixmapPtr pixmap)
+{
+ glamor_egl_destroy_text
[Intel-gfx] [PATCH 0/2] Introduce Glamor to UXA framework.
After discussion with Chris yesterday, I reworked the previous glamor branch and concentrate the patchset to two patches. The first patch is the same as the previous. The second patch merges all the others into one, and made some slightly change. One major change is that I decide to extent the finish_access to handle the flushing operations by check the access type. And also add some missed glamor flushing operations. This patchset initially enable glamor with UXA. And two functions ,fill_spans and poly_fill_rects, go to the glamor path. I tested it with render check, and it works fine. One of the remaining issues is the vsync'ed copy. I discussed with Chris, and already got a solution. Will do that when I enable the copy_area to glamor path. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] glamor: Address GLAMOR/UXA flushing problem.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Tuesday, November 15, 2011 8:25 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Cc: zhigang.g...@linux.intel.com > Subject: Re: [PATCH 2/2] glamor: Address GLAMOR/UXA flushing problem. > > On Tue, 15 Nov 2011 19:36:14 +0800, Zhigang Gong > wrote: > > This commit introduces a new function in UXA layer need_flush which is > > used to let the UXA layer to notify the lower layer that some pixmap > > get modified by GLAMOR. And then the intel driver could know it need > > to flush front buffer latter. > > > > This commit also adds some necessary flushing pointis for UXA layer > > and glamor layer. Basicly, there are three scenarios: > > > > 1. Before calling into glamor layer, it needs to flush all the > > corresponding UXA batch comand buffer. > > > > 2. After calling the glamor rendering functions, it needs to flush the > > pending GL operations. > > > > 3. Before we map a pixmap's BO, we also need to flush all the pending > > GL operations. > > > > The scenario 2 could be eliminated when we fully change to glamor > > path. > > I much prefer the one-sided uxa_prepare_access() you have implemented > here for the glamor side. For completeness, you still probably want the > uxa_finish_access() though. I was thinking about that, but current uxa_finish_access doesn't have a parameter to indicate the access type. I prefer to change the uxa_finish_access's prototype to the same as uxa_prepare_access(). And then will do glamor flush there if the access type is GLAMOR_WRITE. I'm just not very sure whether it is the best way. Will work out a new patch to implement that soon. BTW, May I have your review tag in the commits which get reviewed by you. Thanks. > Alternatively you need to mark up all > operations with uxa_prepare_access(GLAMOR | NATIVE | SW, READ | > WRITE) which then perform the implicit uxa_finish_access() when the > mode changes, and would also allow you to only flush the glamor > operations as required. This looks much complicate than the first solution. I prefer the first solution. -- Zhigang > > Aside from that, this looks like the right approach to handling the flushing. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] glamor: Address GLAMOR/UXA flushing problem.
This commit introduces a new function in UXA layer need_flush
which is used to let the UXA layer to notify the lower layer
that some pixmap get modified by GLAMOR. And then the intel
driver could know it need to flush front buffer latter.
This commit also adds some necessary flushing pointis for UXA
layer and glamor layer. Basicly, there are three scenarios:
1. Before calling into glamor layer, it needs to flush all the
corresponding UXA batch comand buffer.
2. After calling the glamor rendering functions, it needs to
flush the pending GL operations.
3. Before we map a pixmap's BO, we also need to flush all the
pending GL operations.
The scenario 2 could be eliminated when we fully change to glamor
path.
Signed-off-by: Zhigang Gong
---
src/intel_glamor.c | 13 -
src/intel_uxa.c|8 +++-
uxa/uxa-accel.c| 30 --
uxa/uxa.h | 16 +++-
4 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 80cde40..0920c05 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -106,6 +106,16 @@ intel_glamor_destroy_pixmap(PixmapPtr pixmap)
glamor_egl_destroy_textured_pixmap(pixmap);
}
+static void
+intel_glamor_need_flush(DrawablePtr pDrawable)
+{
+ ScrnInfoPtr scrn = xf86Screens[pDrawable->pScreen->myNum];
+ intel_screen_private * intel;
+
+ intel = intel_get_screen_private(scrn);
+ intel->needs_flush = TRUE;
+}
+
Bool
intel_glamor_init(ScreenPtr screen)
{
@@ -113,7 +123,6 @@ intel_glamor_init(ScreenPtr screen)
ScrnInfoPtr scrn = xf86Screens[screen->myNum];
intel_screen_private * intel;
-
if (!glamor_init(screen, GLAMOR_INVERTED_Y_AXIS)) {
xf86DrvMsg(scrn->scrnIndex, X_ERROR,
"Failed to initialize glamor\n");
@@ -130,6 +139,8 @@ intel_glamor_init(ScreenPtr screen)
intel->uxa_driver->flags |= UXA_USE_GLAMOR;
intel->uxa_flags = intel->uxa_driver->flags;
+ intel->uxa_driver->need_flush = intel_glamor_need_flush;
+
xf86DrvMsg(scrn->scrnIndex, X_INFO,
"Use GLAMOR acceleration.\n");
return TRUE;
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 5202076..9fa0165 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -705,8 +705,14 @@ static Bool intel_uxa_prepare_access(PixmapPtr pixmap,
uxa_access_t access)
int ret;
if (!list_is_empty(&priv->batch) &&
- (access == UXA_ACCESS_RW || priv->batch_write))
+ ((access == UXA_ACCESS_RW || access == UXA_GLAMOR_ACCESS_RW)
+|| priv->batch_write))
intel_batch_submit(scrn);
+
+ if (access == UXA_GLAMOR_ACCESS_RW || access == UXA_GLAMOR_ACCESS_RO)
+ return TRUE;
+
+ intel_glamor_block_handler(intel);
if (priv->tiling || bo->size <= intel->max_gtt_map_size)
ret = drm_intel_gem_bo_map_gtt(bo);
diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index 5b27aaa..bd9e02b 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -56,11 +56,16 @@ uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n,
int x1, x2, y;
int off_x, off_y;
- if ((uxa_screen->info->flags & UXA_GLAMOR_FLAGS)
- && (glamor_fill_spans_nf(pDrawable, pGC, n, ppt, pwidth, fSorted)))
- return;
- else if(uxa_screen->info->flags & UXA_USE_GLAMOR_ONLY)
- goto fallback;
+ if ((uxa_screen->info->flags & UXA_GLAMOR_FLAGS)) {
+ uxa_prepare_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ if (glamor_fill_spans_nf(pDrawable,
+pGC, n, ppt, pwidth, fSorted)) {
+ uxa_screen->info->need_flush(pDrawable);
+ glamor_block_handler(screen);
+ return;
+ } else if (uxa_screen->info->flags & UXA_USE_GLAMOR_ONLY)
+ goto fallback;
+ }
if (uxa_screen->swappedOut || uxa_screen->force_fallback)
goto fallback;
@@ -685,9 +690,22 @@ uxa_poly_fill_rect(DrawablePtr pDrawable,
int n;
RegionPtr pReg = RECTS_TO_REGION(pScreen, nrect, prect, CT_UNSORTED);
+ if ((uxa_screen->info->flags & UXA_GLAMOR_FLAGS)) {
+ uxa_prepare_access(pDrawable, UXA_GLAMOR_ACCESS_RW);
+ if (glamor_poly_fill_rect_nf(pDrawable,
+pGC, nrect, prect)) {
+ uxa_screen->info->need_flush(pDrawable);
+ glamor_block_handler(pDrawable->pScreen);
+ return;
+ } else if (uxa_screen->info->flags & UXA_USE_GLAM
[Intel-gfx] [PATCH 1/2] glamor: Move flags check to intel_glamor.
Address Chris comment. We concentrate all the flag check into
intel_glamor layer which makes the interface to glamor
self-contained.
Signed-off-by: Zhigang Gong
---
src/Makefile.am|6 +---
src/intel_driver.c | 25 +
src/intel_glamor.c | 75 +--
src/intel_uxa.c| 31 +
4 files changed, 82 insertions(+), 55 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 1a29390..12ff6ee 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,10 +40,6 @@ SUBDIRS += sna
intel_drv_la_LIBADD += sna/libsna.la
endif
-if GLAMOR
-GLAMOR_SOURCE = intel_glamor.c
-endif
-
NULL:=#
intel_drv_la_SOURCES = \
@@ -74,7 +70,7 @@ intel_drv_la_SOURCES = \
i965_3d.c \
i965_video.c \
i965_render.c \
-$(GLAMOR_SOURCE) \
+intel_glamor.c \
$(NULL)
if DRI
diff --git a/src/intel_driver.c b/src/intel_driver.c
index 0f528f3..2cc418e 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -714,19 +714,12 @@ static Bool I830PreInit(ScrnInfoPtr scrn, int flags)
return FALSE;
}
-#ifdef GLAMOR
-/* Load glamor module */
-if (!xf86LoadSubModule(scrn, "glamor_egl")) {
-PreInitCleanup(scrn);
-return FALSE;
-}
-
-if (!intel_glamor_pre_init(scrn)) {
- xf86DrvMsg(scrn->scrnIndex, X_ERROR,
- "Failed to pre init glamor display.\n");
- return FALSE;
-}
-#endif
+ if (!intel_glamor_pre_init(scrn)) {
+ PreInitCleanup(scrn);
+ xf86DrvMsg(scrn->scrnIndex, X_ERROR,
+ "Failed to pre init glamor display.\n");
+ return FALSE;
+ }
/* Load the dri2 module if requested. */
if (intel->directRenderingType != DRI_DISABLED)
@@ -1126,8 +1119,7 @@ static void I830FreeScreen(int scrnIndex, int flags)
ScrnInfoPtr scrn = xf86Screens[scrnIndex];
intel_screen_private *intel = intel_get_screen_private(scrn);
- if (intel->uxa_flags & UXA_GLAMOR_FLAGS)
- intel_glamor_free_screen(scrnIndex, flags);
+ intel_glamor_free_screen(scrnIndex, flags);
if (intel) {
intel_mode_fini(intel);
@@ -1208,8 +1200,7 @@ static Bool I830CloseScreen(int scrnIndex, ScreenPtr
screen)
DeleteCallback(&FlushCallback, intel_flush_callback, scrn);
- if (intel->uxa_flags & UXA_GLAMOR_FLAGS)
- intel_glamor_close_screen(screen);
+ intel_glamor_close_screen(screen);
if (intel->uxa_driver) {
uxa_driver_fini(screen);
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index cadfc71..80cde40 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -48,6 +48,9 @@ intel_glamor_create_screen_resources(ScreenPtr screen)
ScrnInfoPtr scrn = xf86Screens[screen->myNum];
intel_screen_private *intel = intel_get_screen_private(scrn);
+ if (!(intel->uxa_flags & UXA_GLAMOR_FLAGS))
+ return TRUE;
+
if (!glamor_glyphs_init(screen))
return FALSE;
if (!glamor_egl_create_textured_screen(screen,
@@ -60,57 +63,100 @@ intel_glamor_create_screen_resources(ScreenPtr screen)
Bool
intel_glamor_pre_init(ScrnInfoPtr scrn)
{
+#ifdef GLAMOR
intel_screen_private *intel;
intel = intel_get_screen_private(scrn);
+
+ /* Load glamor module */
+ if (!xf86LoadSubModule(scrn, "glamor_egl"))
+ return FALSE;
+
return glamor_egl_init(scrn, intel->drmSubFD);
+#else
+ return TRUE;
+#endif
}
Bool
intel_glamor_create_textured_pixmap(PixmapPtr pixmap)
{
struct intel_pixmap *priv;
+ ScrnInfoPtr scrn = xf86Screens[pixmap->drawable.pScreen->myNum];
+ intel_screen_private * intel;
+
priv = intel_get_pixmap_private(pixmap);
- return glamor_egl_create_textured_pixmap(pixmap, priv->bo->handle,
-priv->stride);
+ intel = intel_get_screen_private(scrn);
+ if ((intel->uxa_flags & UXA_GLAMOR_FLAGS)
+&& glamor_egl_create_textured_pixmap(pixmap, priv->bo->handle,
+ priv->stride)) {
+ priv->pinned = 1;
+ return TRUE;
+ }
+ return FALSE;
}
void
intel_glamor_destroy_pixmap(PixmapPtr pixmap)
{
- glamor_egl_destroy_textured_pixmap(pixmap);
+ ScrnInfoPtr scrn = xf86Screens[pixmap->drawable.pScreen->myNum];
+ intel_screen_private * intel;
+
+ intel = intel_get_screen_private(scrn);
+ if (intel->uxa_flags & UXA_GLAMOR_FLAGS)
+ glamor_egl_destroy_textured_pixmap(pixmap);
}
Bool
intel_glamor_init(Scree
[Intel-gfx] [PATCH 4/4] glamor: Silence compilation warnings.
As we removed those #ifdef from the body of the code, some
functions are referenced even we haven't defined the GLAMOR
though those functions are never be called at run time. We
still need to inclue those prototypes or make a fake function
to silcent the compilation warnings.
Signed-off-by: Zhigang Gong
---
src/intel_driver.c |2 --
src/intel_uxa.c|2 --
uxa/uxa-accel.c|5 -
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/intel_driver.c b/src/intel_driver.c
index bd57694..0f528f3 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -75,9 +75,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "i915_drm.h"
#include
-#ifdef GLAMOR
#include "intel_glamor.h"
-#endif
/* *INDENT-OFF* */
/*
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index a069980..81d5ec0 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -40,9 +40,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include
#include
-#ifdef GLAMOR
#include "intel_glamor.h"
-#endif
static const int I830CopyROP[16] = {
ROP_0, /* GXclear */
diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index b2e5347..5b27aaa 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -38,7 +38,10 @@
#ifdef GLAMOR
#include "glamor.h"
-#endif
+#else
+#define glamor_fill_spans_nf(...) FALSE
+#define glamor_poly_fill_rect_nf(...) FALSE
+#endif
static void
uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n,
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] glamor: check a flag to indicate whether enable GLAMOR.
According to Chris's comments, this commit try to
elminate #ifdef from the body of the code if possible.
We check the flags to determine whether enable GLAMOR
at runtime, rather than check the MACRO during the compile
time.
Signed-off-by: Zhigang Gong
---
src/intel_driver.c | 12 ++--
src/intel_uxa.c| 38 --
uxa/uxa-accel.c| 16 ++--
3 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/src/intel_driver.c b/src/intel_driver.c
index 63f83e7..bd57694 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -1127,9 +1127,10 @@ static void I830FreeScreen(int scrnIndex, int flags)
{
ScrnInfoPtr scrn = xf86Screens[scrnIndex];
intel_screen_private *intel = intel_get_screen_private(scrn);
-#ifdef GLAMOR
- intel_glamor_free_screen(scrnIndex, flags);
-#endif
+
+ if (intel->uxa_flags & UXA_GLAMOR_FLAGS)
+ intel_glamor_free_screen(scrnIndex, flags);
+
if (intel) {
intel_mode_fini(intel);
intel_close_drm_master(intel);
@@ -1209,9 +1210,8 @@ static Bool I830CloseScreen(int scrnIndex, ScreenPtr
screen)
DeleteCallback(&FlushCallback, intel_flush_callback, scrn);
-#ifdef GLAMOR
- intel_glamor_close_screen(screen);
-#endif
+ if (intel->uxa_flags & UXA_GLAMOR_FLAGS)
+ intel_glamor_close_screen(screen);
if (intel->uxa_driver) {
uxa_driver_fini(screen);
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 3bbe531..a069980 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -968,9 +968,8 @@ void intel_uxa_block_handler(intel_screen_private *intel)
* and beyond rendering results may not hit the
* framebuffer until significantly later.
*/
-#ifdef GLAMOR
- intel_glamor_block_handler(intel);
-#endif
+ if (intel->uxa_flags & UXA_GLAMOR_FLAGS)
+ intel_glamor_block_handler(intel);
intel_flush_rendering(intel);
}
@@ -1102,10 +1101,11 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
list_init(&priv->batch);
list_init(&priv->flush);
intel_set_pixmap_private(pixmap, priv);
-#ifdef GLAMOR
- priv->pinned = 1;
- intel_glamor_create_textured_pixmap(pixmap);
-#endif
+
+ if (intel->uxa_flags & UXA_GLAMOR_FLAGS) {
+ priv->pinned = 1;
+ intel_glamor_create_textured_pixmap(pixmap);
+ }
}
return pixmap;
@@ -1113,10 +1113,11 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
static Bool intel_uxa_destroy_pixmap(PixmapPtr pixmap)
{
+ ScrnInfoPtr scrn = xf86Screens[pixmap->drawable.pScreen->myNum];
+ intel_screen_private *intel = intel_get_screen_private(scrn);
if (pixmap->refcnt == 1) {
-#ifdef GLAMOR
- intel_glamor_destroy_pixmap(pixmap);
-#endif
+ if (intel->uxa_flags & UXA_GLAMOR_FLAGS)
+ intel_glamor_destroy_pixmap(pixmap);
intel_set_pixmap_bo(pixmap, NULL);
}
fbDestroyPixmap(pixmap);
@@ -1149,10 +1150,10 @@ Bool intel_uxa_create_screen_resources(ScreenPtr screen)
scrn->displayWidth = intel->front_pitch / intel->cpp;
}
-#ifdef GLAMOR
- if (!intel_glamor_create_screen_resources(screen))
+ if ((intel->uxa_flags & UXA_GLAMOR_FLAGS)
+ && !intel_glamor_create_screen_resources(screen))
return FALSE;
-#endif
+
return TRUE;
}
@@ -1315,7 +1316,16 @@ Bool intel_uxa_init(ScreenPtr screen)
uxa_set_fallback_debug(screen, intel->fallback_debug);
uxa_set_force_fallback(screen, intel->force_fallback);
#ifdef GLAMOR
- intel_glamor_init(screen);
+ if (intel_glamor_init(screen)) {
+ xf86DrvMsg(scrn->scrnIndex, X_INFO,
+ "Intel GLAMOR initialization successfully.\n");
+ intel->uxa_driver->flags |= UXA_USE_GLAMOR;
+ intel->uxa_flags = intel->uxa_driver->flags;
+ } else {
+ xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+ "Intel GLAMOR initialization failed,"
+ "change to use standard UXA.\n");
+ }
#endif
return TRUE;
}
diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index 18fa63b..b2e5347 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -52,10 +52,12 @@ uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n,
int nbox;
int x1, x2, y;
int off_x, off_y;
-#ifdef GLAMOR
- if (glamor_fill_spans_nf(pDrawable, pGC, n, ppt, pwidth, fSorted))
+
+ if ((uxa_screen->info->flags & UXA_GLAMOR_FLAGS)
+ && (glam
[Intel-gfx] [PATCH 2/4] glamor: Added new data element to track uxa flags in intel structure.
As uxa_driver may be released before the last call to freeScreen
and destroy the last pixmap, we have to introduce a new flag in
intel structure to indicate whether we are using GLAMOR. This
intel->uxa_flag is a clone of intel->uxa_driver->flags element.
Signed-off-by: Zhigang Gong
---
src/intel.h |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/intel.h b/src/intel.h
index 3b3f87d..b24aa02 100644
--- a/src/intel.h
+++ b/src/intel.h
@@ -318,6 +318,7 @@ typedef struct intel_screen_private {
void (*batch_commit_notify) (struct intel_screen_private *intel);
uxa_driver_t *uxa_driver;
+ int uxa_flags;
Bool need_sync;
int accel_pixmap_offset_alignment;
int accel_max_x;
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] glamor: Added flags to indicate whether to use glamor in UXA.
Added two new flags UXA_USE_GLAMOR and UXA_USE_GLAMOR_ONLY.
When UXA_USE_GLAMOR or UXA_USE_GLAMOR_ONLY is set, then it
will use GLAMOR to perform rendering operations by default.
If GLAMOR failed to accelerate the operation and UXA_USE_GLAMOR
is set, it then continue to the normal UXA code path to do
the rendering. If the UXA_USE_GLAMOR_ONLY is set, it then
jump to fallback path directly and avoid the UXA acceleration
code.
Signed-off-by: Zhigang Gong
---
uxa/uxa.h | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/uxa/uxa.h b/uxa/uxa.h
index e001c53..21e6f2a 100644
--- a/uxa/uxa.h
+++ b/uxa/uxa.h
@@ -543,6 +543,22 @@ typedef struct _UxaDriver {
*/
#define UXA_TWO_BITBLT_DIRECTIONS (1 << 2)
+/**
+ * UXA_USE_GLAMOR indicates to use glamor acceleration to perform rendering
+ * as first choice. And if glamor fail to accelerate the rendering, then goto
+ * normal path to do the rendering.
+ */
+#define UXA_USE_GLAMOR (1 << 3)
+
+/**
+ * UXA_USE_GLAMOR_ONLY indicates to use glamor acceleration to perform
rendering.
+ * And if glamor fail to accelerate the rendering, then goto fallback to
+ * use CPU to do the rendering.
+ */
+#define UXA_USE_GLAMOR_ONLY(1 << 4)
+
+#define UXA_GLAMOR_FLAGS (UXA_USE_GLAMOR | UXA_USE_GLAMOR_ONLY)
+
/** @} */
/** @name UXA CreatePixmap hint flags
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Monday, November 14, 2011 5:07 PM
> To: Zhigang Gong; intel-gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
>
> On Mon, 14 Nov 2011 13:01:36 +0800, "Zhigang Gong"
> wrote:
> >
> >
> > > -Original Message-
> > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > > Sent: Friday, November 11, 2011 9:13 PM
> > > To: Zhigang Gong; intel-gfx@lists.freedesktop.org
> > > Subject: RE: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
> > >
> > > On Fri, 11 Nov 2011 18:52:11 +0800, "Zhigang Gong"
> > > wrote:
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > > > > Sent: Friday, November 11, 2011 5:12 PM
> > > > > To: Zhigang Gong; intel-gfx@lists.freedesktop.org
> > > > > Subject: Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
> > > > >
> > > > > On Fri, 11 Nov 2011 16:31:20 +0800, Zhigang Gong
> > > > > wrote:
> > > > > > @@ -965,6 +969,9 @@ void
> > > > > intel_uxa_block_handler(intel_screen_private *intel)
> > > > > > * framebuffer until significantly later.
> > > > > > */
> > > > > > intel_flush_rendering(intel);
> > > > > > +#ifdef GLAMOR
> > > > > > + intel_glamor_block_handler(intel);
> > > > > > +#endif
> > > > > > }
> > > > >
> > > > > I suspect this is the wrong way around as we are not flushing
> > > > > the render cache of glamor's rendering to the scanout until the
> > > > > next block
> > > handler.
> > > > I don't understand here. Would you please explain more detail?
> Thanks.
> > >
> > > Whenever we render, the data ends up in the Render Cache and
> needs
> > > to be flushed out to memory before it is coherent with the CPU or in
> > > this case the Display Engine (i.e. scanout).
> > >
> > > intel_flush_rendering() does two tasks. The first is to submit any
> > > pending batch, and the second is to flush the Render Cache so that
> > > the modifications land on the scanout in a timely manner. It is
> > > probably best
> > if
> > > those two tasks were separated so that we do:
> > >
> > > intel_uxa_block_handler(intel); // flush the UXA batch
> > > intel_glamor_block_handler(intel); // flush the GL batch
> > > intel_flush_rendering(intel); // flush the RenderCache to scanout
> > >
> > > However, you can simply rearrange the code and achieve it with the
> > > existing functions:
> > >
> > > intel_glamor_block_handler(intel); // mark the front bo as dirty
> > > as needbe
> > > intel_flush_rendering(intel); // flush UXA batch along with
> > > RenderCache
> > Thanks for the explanation here. But I still don't think the original
> > code is wrong regard to this cache flushing issue. Here is my
> > analysis:
> > intel_glamor_block_handler calls to glFlush(), and glFlush is similar
> > with the intel_flush_rendering, it calls intel_flush to flush the
> > batch buffers and then call intel_flush_frontbuffer to flush the
> > frontbuffer which flushes the scan out buffer. So when the screen
> > pixmap is accessed by glamor, and after we call
> > intel_glamor_block_handler, the Display Engine should see the correct
> > data
> >
> > Right?
>
> No.
>
> glFlush() does call intel_flush_front(). However that in turn calls
> dri2->flushFrontBuffer which is implemented for EGL with
>
> static void
> dri2_flush_front_buffer(__DRIdrawable * driDrawable, void
> *loaderPrivate) {
>/* FIXME: Does EGL support front buffer rendering at all? */ }
>
> Neither does it perform the intended action via GLX (except that flushing
> the scanout is handled by the DDX as a normal part of its operation).
You are right. EGL layer will not do a really front buffer flushing. We have
to
let it be done in DDX layer. In my version 2 patch set, I already rearrange
the
code sequence as you suggested please review it again. The remaining work
for this issue is that I need to add new code to set the needs_flush
according
to the access type of glamor. Will do that soon.
Thanks.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] glamor: Route fillspans and polyfillrects to glamor.
If GLAMOR is enabled, we route UXA's fillspans and polyfillrects to glamor by default. And if glamor fail to accelerate it, UXA continue to handle it. Reviewed-by: Eugeni Dodonov Signed-off-by: Zhigang Gong --- uxa/uxa-accel.c | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c index 516834f..18fa63b 100644 --- a/uxa/uxa-accel.c +++ b/uxa/uxa-accel.c @@ -27,7 +27,6 @@ *Michel Dänzer * */ - #ifdef HAVE_DIX_CONFIG_H #include #endif @@ -37,6 +36,10 @@ #include "uxa.h" #include "mipict.h" +#ifdef GLAMOR +#include "glamor.h" +#endif + static void uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n, DDXPointPtr ppt, int *pwidth, int fSorted) @@ -49,6 +52,10 @@ uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n, int nbox; int x1, x2, y; int off_x, off_y; +#ifdef GLAMOR + if (glamor_fill_spans_nf(pDrawable, pGC, n, ppt, pwidth, fSorted)) + return; +#endif if (uxa_screen->swappedOut || uxa_screen->force_fallback) goto fallback; @@ -673,6 +680,10 @@ uxa_poly_fill_rect(DrawablePtr pDrawable, int n; RegionPtr pReg = RECTS_TO_REGION(pScreen, nrect, prect, CT_UNSORTED); +#ifdef GLAMOR + if (glamor_poly_fill_rect_nf(pDrawable, pGC, nrect, prect)) + return; +#endif /* Compute intersection of rects and clip region */ REGION_TRANSLATE(pScreen, pReg, pDrawable->x, pDrawable->y); REGION_INTERSECT(pScreen, pReg, pClip, pReg); -- 1.7.4.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/3] glamor: turn on glamor.
Add glamor's initialization to the uxa's control path.
At preInit stage, it creates and initialize EGL display
context for glamor. At the screenInit stage, it initialize
glamor's internal data structures and shaders.
And this commit enables textured pixmap also. Each pixmap
which has a valid BO can get a cooresponding texture.
After this commit. It's ready to do rendering through
glamor's rendering functions.
Reviewed-by: Eugeni Dodonov
Signed-off-by: Zhigang Gong
---
src/intel_driver.c | 26 +-
src/intel_uxa.c| 25 +++--
2 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/src/intel_driver.c b/src/intel_driver.c
index 24696da..63f83e7 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -75,6 +75,10 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "i915_drm.h"
#include
+#ifdef GLAMOR
+#include "intel_glamor.h"
+#endif
+
/* *INDENT-OFF* */
/*
* Note: "ColorKey" is provided for compatibility with the i810 driver.
@@ -712,6 +716,20 @@ static Bool I830PreInit(ScrnInfoPtr scrn, int flags)
return FALSE;
}
+#ifdef GLAMOR
+/* Load glamor module */
+if (!xf86LoadSubModule(scrn, "glamor_egl")) {
+PreInitCleanup(scrn);
+return FALSE;
+}
+
+if (!intel_glamor_pre_init(scrn)) {
+ xf86DrvMsg(scrn->scrnIndex, X_ERROR,
+ "Failed to pre init glamor display.\n");
+ return FALSE;
+}
+#endif
+
/* Load the dri2 module if requested. */
if (intel->directRenderingType != DRI_DISABLED)
xf86LoadSubModule(scrn, "dri2");
@@ -1109,7 +1127,9 @@ static void I830FreeScreen(int scrnIndex, int flags)
{
ScrnInfoPtr scrn = xf86Screens[scrnIndex];
intel_screen_private *intel = intel_get_screen_private(scrn);
-
+#ifdef GLAMOR
+ intel_glamor_free_screen(scrnIndex, flags);
+#endif
if (intel) {
intel_mode_fini(intel);
intel_close_drm_master(intel);
@@ -1189,6 +1209,10 @@ static Bool I830CloseScreen(int scrnIndex, ScreenPtr
screen)
DeleteCallback(&FlushCallback, intel_flush_callback, scrn);
+#ifdef GLAMOR
+ intel_glamor_close_screen(screen);
+#endif
+
if (intel->uxa_driver) {
uxa_driver_fini(screen);
free(intel->uxa_driver);
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 8c6f754..3bbe531 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -40,6 +40,10 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include
#include
+#ifdef GLAMOR
+#include "intel_glamor.h"
+#endif
+
static const int I830CopyROP[16] = {
ROP_0, /* GXclear */
ROP_DSa,/* GXand */
@@ -964,6 +968,9 @@ void intel_uxa_block_handler(intel_screen_private *intel)
* and beyond rendering results may not hit the
* framebuffer until significantly later.
*/
+#ifdef GLAMOR
+ intel_glamor_block_handler(intel);
+#endif
intel_flush_rendering(intel);
}
@@ -1095,6 +1102,10 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
list_init(&priv->batch);
list_init(&priv->flush);
intel_set_pixmap_private(pixmap, priv);
+#ifdef GLAMOR
+ priv->pinned = 1;
+ intel_glamor_create_textured_pixmap(pixmap);
+#endif
}
return pixmap;
@@ -1102,8 +1113,12 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
static Bool intel_uxa_destroy_pixmap(PixmapPtr pixmap)
{
- if (pixmap->refcnt == 1)
+ if (pixmap->refcnt == 1) {
+#ifdef GLAMOR
+ intel_glamor_destroy_pixmap(pixmap);
+#endif
intel_set_pixmap_bo(pixmap, NULL);
+ }
fbDestroyPixmap(pixmap);
return TRUE;
}
@@ -1134,6 +1149,10 @@ Bool intel_uxa_create_screen_resources(ScreenPtr screen)
scrn->displayWidth = intel->front_pitch / intel->cpp;
}
+#ifdef GLAMOR
+ if (!intel_glamor_create_screen_resources(screen))
+ return FALSE;
+#endif
return TRUE;
}
@@ -1295,6 +1314,8 @@ Bool intel_uxa_init(ScreenPtr screen)
uxa_set_fallback_debug(screen, intel->fallback_debug);
uxa_set_force_fallback(screen, intel->force_fallback);
-
+#ifdef GLAMOR
+ intel_glamor_init(screen);
+#endif
return TRUE;
}
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/3] glamor: Initial commit to introduce glamor acceleration.
Added one configuration option --enable-glamor to control
whether use glamor. Added one new file intel_glamor.c to
wrap glamor egl API for intel driver's usage.
This commit doesn't really change the driver's control path.
It just adds necessary files for glamor and change some
configuration.
Reviewed-by: Eugeni Dodonov
Signed-off-by: Zhigang Gong
---
configure.ac | 17 +++
src/Makefile.am|5 ++
src/intel_glamor.c | 130
src/intel_glamor.h | 45 ++
4 files changed, 197 insertions(+), 0 deletions(-)
create mode 100644 src/intel_glamor.c
create mode 100644 src/intel_glamor.h
diff --git a/configure.ac b/configure.ac
index fccab56..dc01c46 100644
--- a/configure.ac
+++ b/configure.ac
@@ -126,6 +126,15 @@ AC_ARG_ENABLE(sna,
[SNA="$enableval"],
[SNA=no])
AM_CONDITIONAL(SNA, test x$SNA != xno)
+
+AC_ARG_ENABLE(glamor,
+ AS_HELP_STRING([--enable-glamor],
+[Enable glamor, a new GL-based acceleration
[default=no]]),
+ [GLAMOR="$enableval"],
+ [GLAMOR=no])
+
+AM_CONDITIONAL(GLAMOR, test x$GLAMOR != xno)
+
AC_MSG_CHECKING([whether to include SNA support])
required_xorg_xserver_version=1.6
@@ -137,6 +146,14 @@ if test "x$SNA" != "xno"; then
fi
AC_MSG_RESULT([$SNA])
+if test "x$GLAMOR" != "xno"; then
+ PKG_CHECK_MODULES(LIBGLAMOR, [glamor])
+ PKG_CHECK_MODULES(LIBGLAMOR_EGL, [glamor-egl])
+ AC_DEFINE(GLAMOR, 1, [Enable glamor acceleration])
+fi
+
+AC_MSG_CHECKING([whether to include GLAMOR support])
+AC_MSG_RESULT([$GLAMOR])
AC_ARG_ENABLE(vmap,
AS_HELP_STRING([--enable-vmap],
diff --git a/src/Makefile.am b/src/Makefile.am
index cd1bb36..1a29390 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,6 +40,10 @@ SUBDIRS += sna
intel_drv_la_LIBADD += sna/libsna.la
endif
+if GLAMOR
+GLAMOR_SOURCE = intel_glamor.c
+endif
+
NULL:=#
intel_drv_la_SOURCES = \
@@ -70,6 +74,7 @@ intel_drv_la_SOURCES = \
i965_3d.c \
i965_video.c \
i965_render.c \
+$(GLAMOR_SOURCE) \
$(NULL)
if DRI
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
new file mode 100644
index 000..cadfc71
--- /dev/null
+++ b/src/intel_glamor.c
@@ -0,0 +1,130 @@
+/*
+ * Copyright © 2011 Intel Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including
+ * the next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *Zhigang Gong
+ *
+ */
+
+#ifdef HAVE_DIX_CONFIG_H
+#include
+#endif
+#include
+#include
+#include
+#include
+#include
+
+#define GLAMOR_FOR_XORG 1
+
+#include
+#include "intel.h"
+#include "intel_glamor.h"
+
+Bool
+intel_glamor_create_screen_resources(ScreenPtr screen)
+{
+ ScrnInfoPtr scrn = xf86Screens[screen->myNum];
+ intel_screen_private *intel = intel_get_screen_private(scrn);
+
+ if (!glamor_glyphs_init(screen))
+ return FALSE;
+ if (!glamor_egl_create_textured_screen(screen,
+ intel->front_buffer->handle,
+ intel->front_pitch))
+ return FALSE;
+ return TRUE;
+}
+
+Bool
+intel_glamor_pre_init(ScrnInfoPtr scrn)
+{
+ intel_screen_private *intel;
+ intel = intel_get_screen_private(scrn);
+ return glamor_egl_init(scrn, intel->drmSubFD);
+}
+
+Bool
+intel_glamor_create_textured_pixmap(PixmapPtr pixmap)
+{
+ struct intel_pixmap *priv;
+ priv = intel_get_pixmap_private(pixmap);
+ return glamor_egl_create_textured_pixmap(pixmap, priv->bo->handle,
+priv->stride);
+}
+
+void
+intel_glamor_destroy_pixmap(PixmapPtr pixmap)
+{
+ glamor_egl_destroy_text
Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
Thanks for the review. Fixed that empty #ifdef block, missed one line code
there to free glamor screen.
From: eugeni.dodo...@gmail.com [mailto:eugeni.dodo...@gmail.com] On Behalf
Of Eugeni Dodonov
Sent: Friday, November 11, 2011 8:59 PM
To: Zhigang Gong
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
On Fri, Nov 11, 2011 at 06:31, Zhigang Gong
wrote:
@@ -1109,7 +1127,8 @@ static void I830FreeScreen(int scrnIndex, int flags)
{
ScrnInfoPtr scrn = xf86Screens[scrnIndex];
intel_screen_private *intel = intel_get_screen_private(scrn);
-
+#ifdef GLAMOR
+#endif
Empty #ifdef block?
--
Eugeni Dodonov <http://eugeni.dodonov.net/>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Glamor update
> -Original Message- > From: Jamey Sharp [mailto:ja...@minilop.net] > Sent: Saturday, November 12, 2011 5:47 AM > To: Zhigang Gong > Cc: xorg-de...@lists.x.org; intel-gfx@lists.freedesktop.org > Subject: Re: Glamor update > > Hello! > > On Fri, Nov 11, 2011 at 04:42:32PM +0800, Zhigang Gong wrote: > > During the last discussion about glamor’s plan in this list, we got a > > conclusion that to extract glamor from xorg and build a separate > > glamor library to be used by any possible DDX driver. And Eric > > suggested I can incrementally merge glamor into Intel video driver. > > Now here is the update. > > > > The separate glamor library is at : > > git://people.freedesktop.org/~gongzg/glamor，it provides two > interfaces: > > > > 1. glamor : Rendering library. All the rendering functions are > > implemented in this package. > > > > 2. glamor-egl : EGL support library. This package provides > functions > > to create and initialize OpenGL/EGL context. > > > > There are a little bit more details to introduce glamor in the README file. > > > > The Intel video driver to merge glamor is at > > git://people.freedesktop.org/~gongzg/xf86-video-intel’s “glamor” > branch. > > > > I just started the merging stage. Only finished 3 patches to enable > > glamor in UXA code path. Currently, only migrate fillspans and > > polyfillrect to glamor. Will continue to migrate the rest functions. > > I've looked over all the source in your glamor repo, and your patches to > the Intel driver. This work looks very promising! I'm eager to see more > progress. > > I don't speak for the Intel graphics team or anything, but I noticed a couple > minor things you might clean up. In the "glamor: turn on glamor." > commit, there's a patch hunk that just adds an empty ifdef: > > - > +#ifdef GLAMOR > +#endif Fixed. > > You might want to remove that. Also, the call to > intel_glamor_create_textured_pixmap in intel_uxa_create_pixmap is > inconsistently indented; it should just be tabs, looks like. Fixed. I will submit the new version patch again latter. Thanks for your carefully reviewing. > > Any chance you'll add some of the Xephyr/glamor functionality to > xf86-video-nested, too? I think that should be easier to test and hack on. Currently, we don't have enough bandwidth to do that. If anyone in the community want to do that, I'd be happy to provide help. > > Jamey ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, November 11, 2011 9:13 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor. > > On Fri, 11 Nov 2011 18:52:11 +0800, "Zhigang Gong" > wrote: > > > > > > > -Original Message- > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > Sent: Friday, November 11, 2011 5:12 PM > > > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor. > > > > > > On Fri, 11 Nov 2011 16:31:20 +0800, Zhigang Gong > > > wrote: > > > > @@ -965,6 +969,9 @@ void > > > intel_uxa_block_handler(intel_screen_private *intel) > > > > * framebuffer until significantly later. > > > > */ > > > > intel_flush_rendering(intel); > > > > +#ifdef GLAMOR > > > > + intel_glamor_block_handler(intel); > > > > +#endif > > > > } > > > > > > I suspect this is the wrong way around as we are not flushing the > > > render cache of glamor's rendering to the scanout until the next block > handler. > > I don't understand here. Would you please explain more detail? Thanks. > > Whenever we render, the data ends up in the Render Cache and needs to > be flushed out to memory before it is coherent with the CPU or in this > case the Display Engine (i.e. scanout). > > intel_flush_rendering() does two tasks. The first is to submit any pending > batch, and the second is to flush the Render Cache so that the > modifications land on the scanout in a timely manner. It is probably best if > those two tasks were separated so that we do: > > intel_uxa_block_handler(intel); // flush the UXA batch > intel_glamor_block_handler(intel); // flush the GL batch > intel_flush_rendering(intel); // flush the RenderCache to scanout > > However, you can simply rearrange the code and achieve it with the > existing functions: > > intel_glamor_block_handler(intel); // mark the front bo as dirty as > needbe > intel_flush_rendering(intel); // flush UXA batch along with RenderCache Thanks for the explanation here. But I still don't think the original code is wrong regard to this cache flushing issue. Here is my analysis: intel_glamor_block_handler calls to glFlush(), and glFlush is similar with the intel_flush_rendering, it calls intel_flush to flush the batch buffers and then call intel_flush_frontbuffer to flush the frontbuffer which flushes the scan out buffer. So when the screen pixmap is accessed by glamor, and after we call intel_glamor_block_handler, the Display Engine should see the correct data Right? - Zhigang > > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] glamor: Route fillspans and polyfillrects to glamor.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, November 11, 2011 9:55 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH 3/3] glamor: Route fillspans and polyfillrects > to glamor. > > On Fri, 11 Nov 2011 18:48:57 +0800, "Zhigang Gong" > wrote: > > > -Original Message- > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > Sent: Friday, November 11, 2011 5:08 PM > > > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCH 3/3] glamor: Route fillspans and > > polyfillrects > > > to glamor. > > > > > > On Fri, 11 Nov 2011 16:31:21 +0800, Zhigang Gong > > > wrote: > > > > If GLAMOR is enabled, we route UXA's fillspans and polyfillrects > > > > to glamor by default. And if glamor fail to accelerate it, UXA > > > > continue to handle it. > > > > > > How is serialisation handled between the UXA and glamor acceleration > > > routines? Don't you need to flush the UXA batch (if the pixmap is > > > active) before handing over to glamor and similarly flush the glamor > > > pixmap after failure? > > Thanks for pointing this issue out. This is something I want to be > > discussed here. > > > > There are three types of access to the pixmap: > > 1. UXA batch command buffer. > > 2. Glamor through OpenGL > > 3. CPU access mapped BO buffer. > > > > My understanding is that the leading two types has the queue > mechanism > > and need to be handled carefully. In general, we can treat glamor 's > > access as another batch buffer. Then in the place where current intel > > driver need to flush UXA batch buffer, we also need to flush the GL > > operations there. Right? > > > > And besides above places we need to flush glamor, we also need to > > flush UXA batch buffer before call into glamor and also need to flush > > glamor after the glamor rendering function really touch the pixmap. > > Right, we want to consider it as another form of pixmap migration, just > instead of between different regions of memory we are migrating > between different queues. We could envision using fences to coordinate > rendering between the different batches, but that is likely to be overkill > and not possible with most Intel hardware. Understand, so we can't expect to solve this overhead problem that way. IMO, as eventually, glamor will take over all the rendering functions, if glamor is enabled. This heavy flushing may be get eliminated then. Anyway, currently, I still need to work out another patch to handle all the needed flushing OPs. Will submit it to review soon. - Zhigang > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, November 11, 2011 5:12 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor. > > On Fri, 11 Nov 2011 16:31:20 +0800, Zhigang Gong > wrote: > > @@ -965,6 +969,9 @@ void > intel_uxa_block_handler(intel_screen_private *intel) > > * framebuffer until significantly later. > > */ > > intel_flush_rendering(intel); > > +#ifdef GLAMOR > > + intel_glamor_block_handler(intel); > > +#endif > > } > > I suspect this is the wrong way around as we are not flushing the render > cache of glamor's rendering to the scanout until the next block handler. I don't understand here. Would you please explain more detail? Thanks. > > In general, try to keep the #ifdef out of the body of the code. In this case, > and others, make intel_glamor_block_handler() be a no-op if GLAMOR is > not enabled. Agreed, will fix it next version. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] glamor: Route fillspans and polyfillrects to glamor.
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, November 11, 2011 5:08 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 3/3] glamor: Route fillspans and polyfillrects > to glamor. > > On Fri, 11 Nov 2011 16:31:21 +0800, Zhigang Gong > wrote: > > If GLAMOR is enabled, we route UXA's fillspans and polyfillrects to > > glamor by default. And if glamor fail to accelerate it, UXA continue > > to handle it. > > How is serialisation handled between the UXA and glamor acceleration > routines? Don't you need to flush the UXA batch (if the pixmap is active) > before handing over to glamor and similarly flush the glamor pixmap after > failure? Thanks for pointing this issue out. This is something I want to be discussed here. There are three types of access to the pixmap: 1. UXA batch command buffer. 2. Glamor through OpenGL 3. CPU access mapped BO buffer. My understanding is that the leading two types has the queue mechanism and need to be handled carefully. In general, we can treat glamor 's access as another batch buffer. Then in the place where current intel driver need to flush UXA batch buffer, we also need to flush the GL operations there. Right? And besides above places we need to flush glamor, we also need to flush UXA batch buffer before call into glamor and also need to flush glamor after the glamor rendering function really touch the pixmap. Any comments? > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] glamor: Route fillspans and polyfillrects to glamor.
If GLAMOR is enabled, we route UXA's fillspans and polyfillrects to glamor by default. And if glamor fail to accelerate it, UXA continue to handle it. Signed-off-by: Zhigang Gong --- uxa/uxa-accel.c | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c index 516834f..18fa63b 100644 --- a/uxa/uxa-accel.c +++ b/uxa/uxa-accel.c @@ -27,7 +27,6 @@ *Michel Dänzer * */ - #ifdef HAVE_DIX_CONFIG_H #include #endif @@ -37,6 +36,10 @@ #include "uxa.h" #include "mipict.h" +#ifdef GLAMOR +#include "glamor.h" +#endif + static void uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n, DDXPointPtr ppt, int *pwidth, int fSorted) @@ -49,6 +52,10 @@ uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n, int nbox; int x1, x2, y; int off_x, off_y; +#ifdef GLAMOR + if (glamor_fill_spans_nf(pDrawable, pGC, n, ppt, pwidth, fSorted)) + return; +#endif if (uxa_screen->swappedOut || uxa_screen->force_fallback) goto fallback; @@ -673,6 +680,10 @@ uxa_poly_fill_rect(DrawablePtr pDrawable, int n; RegionPtr pReg = RECTS_TO_REGION(pScreen, nrect, prect, CT_UNSORTED); +#ifdef GLAMOR + if (glamor_poly_fill_rect_nf(pDrawable, pGC, nrect, prect)) + return; +#endif /* Compute intersection of rects and clip region */ REGION_TRANSLATE(pScreen, pReg, pDrawable->x, pDrawable->y); REGION_INTERSECT(pScreen, pReg, pClip, pReg); -- 1.7.4.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] glamor: turn on glamor.
Add glamor's initialization to the uxa's control path.
At preInit stage, it creates and initialize EGL display
context for glamor. At the screenInit stage, it initialize
glamor's internal data structures and shaders.
And this commit enables textured pixmap also. Each pixmap
which has a valid BO can get a cooresponding texture.
After this commit. It's ready to do rendering through
glamor's rendering functions.
Signed-off-by: Zhigang Gong
---
src/intel_driver.c | 25 -
src/intel_uxa.c| 22 +-
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/intel_driver.c b/src/intel_driver.c
index 24696da..cf50d67 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -75,6 +75,10 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "i915_drm.h"
#include
+#ifdef GLAMOR
+#include "intel_glamor.h"
+#endif
+
/* *INDENT-OFF* */
/*
* Note: "ColorKey" is provided for compatibility with the i810 driver.
@@ -712,6 +716,20 @@ static Bool I830PreInit(ScrnInfoPtr scrn, int flags)
return FALSE;
}
+#ifdef GLAMOR
+/* Load glamor module */
+if (!xf86LoadSubModule(scrn, "glamor_egl")) {
+PreInitCleanup(scrn);
+return FALSE;
+}
+
+if (!intel_glamor_pre_init(scrn)) {
+ xf86DrvMsg(scrn->scrnIndex, X_ERROR,
+ "Failed to pre init glamor display.\n");
+ return FALSE;
+}
+#endif
+
/* Load the dri2 module if requested. */
if (intel->directRenderingType != DRI_DISABLED)
xf86LoadSubModule(scrn, "dri2");
@@ -1109,7 +1127,8 @@ static void I830FreeScreen(int scrnIndex, int flags)
{
ScrnInfoPtr scrn = xf86Screens[scrnIndex];
intel_screen_private *intel = intel_get_screen_private(scrn);
-
+#ifdef GLAMOR
+#endif
if (intel) {
intel_mode_fini(intel);
intel_close_drm_master(intel);
@@ -1189,6 +1208,10 @@ static Bool I830CloseScreen(int scrnIndex, ScreenPtr
screen)
DeleteCallback(&FlushCallback, intel_flush_callback, scrn);
+#ifdef GLAMOR
+ intel_glamor_close_screen(screen);
+#endif
+
if (intel->uxa_driver) {
uxa_driver_fini(screen);
free(intel->uxa_driver);
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 8c6f754..f0e5803 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -40,6 +40,10 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include
#include
+#ifdef GLAMOR
+#include "intel_glamor.h"
+#endif
+
static const int I830CopyROP[16] = {
ROP_0, /* GXclear */
ROP_DSa,/* GXand */
@@ -965,6 +969,9 @@ void intel_uxa_block_handler(intel_screen_private *intel)
* framebuffer until significantly later.
*/
intel_flush_rendering(intel);
+#ifdef GLAMOR
+ intel_glamor_block_handler(intel);
+#endif
}
static PixmapPtr
@@ -1095,6 +1102,10 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
list_init(&priv->batch);
list_init(&priv->flush);
intel_set_pixmap_private(pixmap, priv);
+#ifdef GLAMOR
+ priv->pinned = 1;
+intel_glamor_create_textured_pixmap(pixmap);
+#endif
}
return pixmap;
@@ -1102,8 +1113,12 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h,
int depth,
static Bool intel_uxa_destroy_pixmap(PixmapPtr pixmap)
{
- if (pixmap->refcnt == 1)
+ if (pixmap->refcnt == 1) {
+#ifdef GLAMOR
+ intel_glamor_destroy_pixmap(pixmap);
+#endif
intel_set_pixmap_bo(pixmap, NULL);
+ }
fbDestroyPixmap(pixmap);
return TRUE;
}
@@ -1134,6 +1149,9 @@ Bool intel_uxa_create_screen_resources(ScreenPtr screen)
scrn->displayWidth = intel->front_pitch / intel->cpp;
}
+ if (!intel_glamor_create_screen_resources(screen))
+ return FALSE;
+
return TRUE;
}
@@ -1296,5 +1314,7 @@ Bool intel_uxa_init(ScreenPtr screen)
uxa_set_fallback_debug(screen, intel->fallback_debug);
uxa_set_force_fallback(screen, intel->force_fallback);
+ intel_glamor_init(screen);
+
return TRUE;
}
--
1.7.4.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] glamor: Initial commit to introduce glamor acceleration.
Added one configuration option --enable-glamor to control
whether use glamor. Added one new file intel_glamor.c to
wrap glamor egl API for intel driver's usage.
This commit doesn't really change the driver's control path.
It just adds necessary files for glamor and change some
configuration.
Signed-off-by: Zhigang Gong
---
configure.ac | 17 +++
src/Makefile.am|5 ++
src/intel_glamor.c | 130
src/intel_glamor.h | 45 ++
4 files changed, 197 insertions(+), 0 deletions(-)
create mode 100644 src/intel_glamor.c
create mode 100644 src/intel_glamor.h
diff --git a/configure.ac b/configure.ac
index fccab56..dc01c46 100644
--- a/configure.ac
+++ b/configure.ac
@@ -126,6 +126,15 @@ AC_ARG_ENABLE(sna,
[SNA="$enableval"],
[SNA=no])
AM_CONDITIONAL(SNA, test x$SNA != xno)
+
+AC_ARG_ENABLE(glamor,
+ AS_HELP_STRING([--enable-glamor],
+[Enable glamor, a new GL-based acceleration
[default=no]]),
+ [GLAMOR="$enableval"],
+ [GLAMOR=no])
+
+AM_CONDITIONAL(GLAMOR, test x$GLAMOR != xno)
+
AC_MSG_CHECKING([whether to include SNA support])
required_xorg_xserver_version=1.6
@@ -137,6 +146,14 @@ if test "x$SNA" != "xno"; then
fi
AC_MSG_RESULT([$SNA])
+if test "x$GLAMOR" != "xno"; then
+ PKG_CHECK_MODULES(LIBGLAMOR, [glamor])
+ PKG_CHECK_MODULES(LIBGLAMOR_EGL, [glamor-egl])
+ AC_DEFINE(GLAMOR, 1, [Enable glamor acceleration])
+fi
+
+AC_MSG_CHECKING([whether to include GLAMOR support])
+AC_MSG_RESULT([$GLAMOR])
AC_ARG_ENABLE(vmap,
AS_HELP_STRING([--enable-vmap],
diff --git a/src/Makefile.am b/src/Makefile.am
index cd1bb36..1a29390 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,6 +40,10 @@ SUBDIRS += sna
intel_drv_la_LIBADD += sna/libsna.la
endif
+if GLAMOR
+GLAMOR_SOURCE = intel_glamor.c
+endif
+
NULL:=#
intel_drv_la_SOURCES = \
@@ -70,6 +74,7 @@ intel_drv_la_SOURCES = \
i965_3d.c \
i965_video.c \
i965_render.c \
+$(GLAMOR_SOURCE) \
$(NULL)
if DRI
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
new file mode 100644
index 000..cadfc71
--- /dev/null
+++ b/src/intel_glamor.c
@@ -0,0 +1,130 @@
+/*
+ * Copyright © 2011 Intel Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including
+ * the next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *Zhigang Gong
+ *
+ */
+
+#ifdef HAVE_DIX_CONFIG_H
+#include
+#endif
+#include
+#include
+#include
+#include
+#include
+
+#define GLAMOR_FOR_XORG 1
+
+#include
+#include "intel.h"
+#include "intel_glamor.h"
+
+Bool
+intel_glamor_create_screen_resources(ScreenPtr screen)
+{
+ ScrnInfoPtr scrn = xf86Screens[screen->myNum];
+ intel_screen_private *intel = intel_get_screen_private(scrn);
+
+ if (!glamor_glyphs_init(screen))
+ return FALSE;
+ if (!glamor_egl_create_textured_screen(screen,
+ intel->front_buffer->handle,
+ intel->front_pitch))
+ return FALSE;
+ return TRUE;
+}
+
+Bool
+intel_glamor_pre_init(ScrnInfoPtr scrn)
+{
+ intel_screen_private *intel;
+ intel = intel_get_screen_private(scrn);
+ return glamor_egl_init(scrn, intel->drmSubFD);
+}
+
+Bool
+intel_glamor_create_textured_pixmap(PixmapPtr pixmap)
+{
+ struct intel_pixmap *priv;
+ priv = intel_get_pixmap_private(pixmap);
+ return glamor_egl_create_textured_pixmap(pixmap, priv->bo->handle,
+priv->stride);
+}
+
+void
+intel_glamor_destroy_pixmap(PixmapPtr pixmap)
+{
+ glamor_egl_destroy_textured_pixmap(pixmap);
[Intel-gfx] Glamor update
Hi folks， During the last discussion about glamor’s plan in this list, we got a conclusion that to extract glamor from xorg and build a separate glamor library to be used by any possible DDX driver. And Eric suggested I can incrementally merge glamor into Intel video driver. Now here is the update. The separate glamor library is at : git://people.freedesktop.org/~gongzg/glamor，it provides two interfaces: 1. glamor : Rendering library. All the rendering functions are implemented in this package. 2. glamor-egl : EGL support library. This package provides functions to create and initialize OpenGL/EGL context. There are a little bit more details to introduce glamor in the README file. The Intel video driver to merge glamor is at git://people.freedesktop.org/~gongzg/xf86-video-intel’s “glamor” branch. I just started the merging stage. Only finished 3 patches to enable glamor in UXA code path. Currently, only migrate fillspans and polyfillrect to glamor. Will continue to migrate the rest functions. I will submit the patches to intel-gfx mail list soon. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Glamor : a 2D rendering engine based on OpenGL for xserver
Here are some preliminary performance data I got by using cairo performance trace and x11perf on three different machines. We can see on SNB GT1 glamor can get identical performance with UXA, and on Ironlake machine, glamor get identical performance with SNA. But on SNB GT2 machine, glamor's performance is much slower than UXA and SNA. Need further effort to understand and further tune glamor on SNB platform. rgb10text(max/min) aa10text(max/min) firefox-planet(xcb) - cairo-trace (higher is better) (higher is better) (lower is better) CPU : i5-2500K @ 3.30GHz GPU: ( SNB GT2) glamor 1.16M1.37M10s sna 2.19M/1.95M2.88M/2.41M4.2s uxa 2.49M 3.11M5.1s cairo-image N/A N/A 9.5s CPU: i7-2600 @3.4GHz GPU: (SNB GT1) glamor 1.21M 1.42M 10.1s sna 2.12M/1.74M2.58M/2.26M 10.3s uxa 1.2M 1.39M11.3s cairo-image N/AN/A 7.5s CPU: i5-480 @2.2GHz GPU: (Ironlake) glamor 772K 913K18.7s sna 780K/1.02M1.19M/1.34M 18.7s uxa 693K 935K 32.9s cairo-image N/AN/A 15.3s On Wed, Aug 17, 2011 at 8:18 PM, zhigang gong wrote: > Hi folks, > > Glamor project was initiated by Eric at last year which is to implement a new > rendering acceleration engine by using OpenGL to handle all the 2D operations. > It is somehow very similar with UXA. The major difference is that we use > texture > to hold pixmap if possible, and then use shaders to fill/composite > those pixmaps, > but UXA is calling libdrm to do buffer allocation and prepare/submit batch > buffer to GPU to start the rendering. > > Now I'm working on this project, and add one ddx driver based on MESA/EGL > for the Xorg server. We have two experimental xservers currently, one is Xorg > with glamor ddx driver and the other is Xephyr. KRH suggested me to add GLES2 > support, now it's almost done. Glamor xserver could run with GLES2 with major > color format support. Now it’s ready to move to wayland. > > After some performance tuning, we now can avoid most of the fallback path > for common desktop applications. On some old platforms we can get identical > or even better performance with UXA/SNA, for example on a i5-480M machine > with GMA graphic chip. But on the latest SNB platform, there is still a big > performance gap. On latest SNB machine,glamor can only get about 1/2 – > 1/3 performance compare to UXA/SNA. The glyph rendering is the bottle-neck > will further tune this part’s performance. > > I want to get some comments here from who may be interested in glamor > before I move forward. From my point of view, if we can continue to improve > glamor's performance, and to achieve identical or even better :) performance > with uxa/sna then glamor could be a good candidate of new device’s ddx driver > and can save many effort of writing new 2D drivers for each new hardware. > Key point here is the performance. I discussed with Eric and we just saw one > big limitation is the texture size limitation. Is there any other > serious resctriction > here? > > Here is a draft TODO list for glamor. Ask for comments here too. > Actually, Eric already told me some things I need to do. adjust the > priority according > to his suggestion. Thanks Eric. > > 0. Ongoing effort: > Enable glx/dri2 support. Need to handle the communication between dri2 > buffer and textures. One possible solution is to create a EGLImage from > the dri2 buffer object's handle. And then bind the image to a texture. > > 1. Testing > Verify with X test suite and rendercheck and make sure no regression. > > 2. Submit patchset to Xorg. > Not sure how to do this, who should I contact first? The patchset has > three > parts logically, one is a glamor directory holds the hardware independent > rendering code. The second part is a patch to Xephyr which is to enable > the > xephyr to use glamor. The third part is a simple ddx driver for xorg. > > 3. Further performance tuning. > Optimize glyphs rendering. > Current glyphs rendering code is based on a very old uxa > implementation, > we should have some chance to optimize this part. > Large pixmap support. > Mesa only support rendering 4096x4096 texture and some OpenGL/ES2 > even only support 2048x2048. This will enforce us fallback to use CPU >
[Intel-gfx] [RFC] Glamor : a 2D rendering engine based on OpenGL for xserver
Hi folks, Glamor project was initiated by Eric at last year which is to implement a new rendering acceleration engine by using OpenGL to handle all the 2D operations. It is somehow very similar with UXA. The major difference is that we use texture to hold pixmap if possible, and then use shaders to fill/composite those pixmaps, but UXA is calling libdrm to do buffer allocation and prepare/submit batch buffer to GPU to start the rendering. Now I'm working on this project, and add one ddx driver based on MESA/EGL for the Xorg server. We have two experimental xservers currently, one is Xorg with glamor ddx driver and the other is Xephyr. KRH suggested me to add GLES2 support, now it's almost done. Glamor xserver could run with GLES2 with major color format support. Now it’s ready to move to wayland. After some performance tuning, we now can avoid most of the fallback path for common desktop applications. On some old platforms we can get identical or even better performance with UXA/SNA, for example on a i5-480M machine with GMA graphic chip. But on the latest SNB platform, there is still a big performance gap. On latest SNB machine,glamor can only get about 1/2 – 1/3 performance compare to UXA/SNA. The glyph rendering is the bottle-neck will further tune this part’s performance. I want to get some comments here from who may be interested in glamor before I move forward. From my point of view, if we can continue to improve glamor's performance, and to achieve identical or even better :) performance with uxa/sna then glamor could be a good candidate of new device’s ddx driver and can save many effort of writing new 2D drivers for each new hardware. Key point here is the performance. I discussed with Eric and we just saw one big limitation is the texture size limitation. Is there any other serious resctriction here? Here is a draft TODO list for glamor. Ask for comments here too. Actually, Eric already told me some things I need to do. adjust the priority according to his suggestion. Thanks Eric. 0. Ongoing effort: Enable glx/dri2 support. Need to handle the communication between dri2 buffer and textures. One possible solution is to create a EGLImage from the dri2 buffer object's handle. And then bind the image to a texture. 1. Testing Verify with X test suite and rendercheck and make sure no regression. 2. Submit patchset to Xorg. Not sure how to do this, who should I contact first? The patchset has three parts logically, one is a glamor directory holds the hardware independent rendering code. The second part is a patch to Xephyr which is to enable the xephyr to use glamor. The third part is a simple ddx driver for xorg. 3. Further performance tuning. Optimize glyphs rendering. Current glyphs rendering code is based on a very old uxa implementation, we should have some chance to optimize this part. Large pixmap support. Mesa only support rendering 4096x4096 texture and some OpenGL/ES2 even only support 2048x2048. This will enforce us fallback to use CPU memory which is very slow. I'm planning to use multiple texture to represent one single large pixmap. Fully optimize the gradient filling by using shader. Support all compositing OPs. Move 1bpp to texture (currently fallback to cpu memory). Support all color format for GLES2. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] EGL: Add hardware cursor image format for EGL image extension.
Current EGL only support one drm buffer format which is
EGL_DRM_BUFFER_FORMAT_ARGB32_MESA. This
format works fine with normal image, but if we want to
create a drm buffer which will be used for a hardware cursor,
then it has problem. The default behaviou for this format is to
enable tiling for i915 driver. Thus it will choose 512 as the buffer's
stride. But the hardware cursor can only support 256 as the
row stride. To solve this problem, I create a new drm buffer
format named EGL_DRM_BUFFER_FORMAT_HWCURSOR_ARGB32_MESA
for hardware cursor usage. It will disable the tiling by default
for i915 driver and then choose 256 as its pitch value. I tested it on a
Sandy bridge platform, it works fine.
---
include/EGL/eglext.h |1 +
include/GL/internal/dri_interface.h |1 +
src/egl/drivers/dri2/egl_dri2.c |3 +++
src/mesa/drivers/dri/intel/intel_screen.c |7 +--
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/EGL/eglext.h b/include/EGL/eglext.h
index 9fd3eb8..d0153d3 100644
--- a/include/EGL/eglext.h
+++ b/include/EGL/eglext.h
@@ -127,6 +127,7 @@ typedef EGLBoolean (EGLAPIENTRYP
PFNEGLDESTROYIMAGEKHRPROC) (EGLDisplay dpy, EGL
/* EGL_DRM_BUFFER_FORMAT_MESA tokens */
#define EGL_DRM_BUFFER_FORMAT_ARGB32_MESA 0x31D2
+#define EGL_DRM_BUFFER_FORMAT_HWCURSOR_ARGB32_MESA 0x31D3
/* EGL_DRM_BUFFER_USE_MESA bits */
#define EGL_DRM_BUFFER_USE_SCANOUT_MESA0x0001
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 2fb729a..fa2341b 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/interna
@@ -127,6 +127,7 @@ typedef EGLBoolean (EGLAPIENTRYP
PFNEGLDESTROYIMAGEKHRPROC) (EGLDisplay dpy, EGL
/* EGL_DRM_BUFFER_FORMAT_MESA tokens */
#define EGL_DRM_BUFFER_FORMAT_ARGB32_MESA 0x31D2
+#define EGL_DRM_BUFFER_FORMAT_HWCURSOR_ARGB32_MESA 0x31D3
/* EGL_DRM_BUFFER_USE_MESA bits */
#define EGL_DRM_BUFFER_USE_SCANOUT_MESA0x0001
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 2fb729a..fa2341b 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -813,6 +813,7 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_FORMAT_RGB565 0x1001
#define __DRI_IMAGE_FORMAT_XRGB 0x1002
#define __DRI_IMAGE_FORMAT_ARGB 0x1003
+#define __DRI_IMAGE_FORMAT_HWCURSOR_ARGB 0x1004
#define __DRI_IMAGE_USE_SHARE 0x0001
#define __DRI_IMAGE_USE_SCANOUT0x0002
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 5e47fbe..23ab0ed 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1043,6 +1043,9 @@ dri2_create_drm_image_mesa(_EGLDriver *drv,
_EGLDisplay *disp,
}
switch (attrs.DRMBufferFormatMESA) {
+ case EGL_DRM_BUFFER_FORMAT_HWCURSOR_ARGB32_MESA:
+ format = __DRI_IMAGE_FORMAT_HWCURSOR_ARGB;
+ break;
case EGL_DRM_BUFFER_FORMAT_ARGB32_MESA:
format = __DRI_IMAGE_FORMAT_ARGB;
break;
diff --git a/src/mesa/drivers/dri/intel/intel_screen.c
b/src/mesa/drivers/dri/intel/intel_screen.c
index 7de0d12..475c142 100644
--- a/src/mesa/drivers/dri/intel/intel_screen.c
+++ b/src/mesa/drivers/dri/intel/intel_screen.c
@@ -217,11 +217,12 @@ intel_create_image(__DRIscreen *screen,
__DRIimage *image;
struct intel_screen *intelScreen = screen->private;
int cpp;
+ int tiling;
image = CALLOC(sizeof *image);
if (image == NULL)
return NULL;
-
+ tiling = I915_TILING_X;
switch (format) {
case __DRI_IMAGE_FORMAT_RGB565:
image->format = MESA_FORMAT_RGB565;
@@ -233,6 +234,8 @@ intel_create_image(__DRIscreen *screen,
image->internal_format = GL_RGB;
image->data_type = GL_UNSIGNED_BYTE;
break;
+ case __DRI_IMAGE_FORMAT_HWCURSOR_ARGB:
+ tiling = I915_TILING_NONE;
case __DRI_IMAGE_FORMAT_ARGB:
image->format = MESA_FORMAT_ARGB;
image->internal_format = GL_RGBA;
@@ -247,7 +250,7 @@ intel_create_image(__DRIscreen *screen,
cpp = _mesa_get_format_bytes(image->format);
image->region =
- intel_region_alloc(intelScreen, I915_TILING_X,
+ intel_region_alloc(intelScreen, tiling,
cpp, width, height, GL_TRUE);
if (image->region == NULL) {
FREE(image);
--
1.7.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
