Re: [Freedreno] [PATCH 2/2] drm/msm: Add MSM_WAIT_IOVA ioctl
On Tue, Jan 14, 2020 at 9:23 AM Jordan Crouse wrote: > > On Tue, Jan 14, 2020 at 08:52:43AM -0800, Rob Clark wrote: > > On Mon, Jan 13, 2020 at 9:51 AM Jordan Crouse > > wrote: > > > > > > On Mon, Jan 13, 2020 at 10:36:05AM -0500, Brian Ho wrote: > > > > + > > > > + vaddr = base_vaddr + args->offset; > > > > + > > > > + /* Assumes WC mapping */ > > > > + ret = wait_event_interruptible_timeout( > > > > + gpu->event, *vaddr >= args->value, > > > > remaining_jiffies); > > > > > > I feel like a barrier might be needed before checking *vaddr just in case > > > you > > > get the interrupt and wake up the queue before the write posts from the > > > hardware. > > > > > > > if the gpu is doing posted (or cached) writes, I don't think there is > > even a CPU side barrier primitive that could wait for that? I think > > we rely on the GPU not interrupting the CPU until the write is posted > > Once the GPU puts the write on the bus then it is up to the whims of the CPU > architecture. If the writes are being done out of order you run a chance of > firing the interrupt and making it all the way to your handler before the > writes > catch up. > > Since you are scheduling and doing a bunch of things in between you probably > don't need to worry but if you start missing events and you don't know why > then > this could be why. A rmb() would give you piece of mind at the cost of being > Yet Another Barrier (TM). Doesn't the fence case do the same thing without a barrier? > Jordan > > > BR, > > -R > > ___ > > Freedreno mailing list > > freedr...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/freedreno > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/msm: Use DRM_DEV_INFO_RATELIMITED for shrinker messages
On Mon, Jan 21, 2019 at 1:36 AM Jani Nikula wrote:
>
> On Fri, 18 Jan 2019, "Kristian H. Kristensen" wrote:
> > Otherwise we get hard to track down "Purging: 123123 bytes" messages in
> > the log.
> >
> > Signed-off-by: Kristian H. Kristensen
> > ---
> > drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c
> > b/drivers/gpu/drm/msm/msm_gem_shrinker.c
> > index b72d8e6cd51d7..8161923892f55 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
> > @@ -98,7 +98,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct
> > shrink_control *sc)
> > mutex_unlock(&dev->struct_mutex);
> >
> > if (freed > 0)
> > - pr_info_ratelimited("Purging %lu bytes\n", freed <<
> > PAGE_SHIFT);
> > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %lu bytes\n",
> > freed << PAGE_SHIFT);
>
> I'm not opposed to the patches per se, but it does seem a bit odd to be
> printing info level messages in a way that might need ratelimiting. Is
> this a hint you should perhaps make it debug?
Yeah, that's a good point - maybe this just needs to be debug...
Kristian
> BR,
> Jani.
>
>
> >
> > return freed;
> > }
> > @@ -134,7 +134,7 @@ msm_gem_shrinker_vmap(struct notifier_block *nb,
> > unsigned long event, void *ptr)
> > *(unsigned long *)ptr += unmapped;
> >
> > if (unmapped > 0)
> > - pr_info_ratelimited("Purging %u vmaps\n", unmapped);
> > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %u vmaps\n",
> > unmapped);
> >
> > return NOTIFY_DONE;
> > }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Mesa-dev] Allocator Nouveau driver, Mesa EXT_external_objects, and DRM metadata import interfaces
On Wed, Dec 20, 2017 at 12:41 PM, Miguel Angel Vico wrote: > Inline. > > On Wed, 20 Dec 2017 11:54:10 -0800 > Kristian Høgsberg wrote: > > > On Wed, Dec 20, 2017 at 11:51 AM, Daniel Vetter wrote: > > > Since this also involves the kernel let's add dri-devel ... > > Yeah, I forgot. Thanks Daniel! > > > > > > > On Wed, Dec 20, 2017 at 5:51 PM, Miguel Angel Vico < > mvicom...@nvidia.com> wrote: > > >> Hi all, > > >> > > >> As many of you already know, I've been working with James Jones on the > > >> Generic Device Allocator project lately. He started a discussion > thread > > >> some weeks ago seeking feedback on the current prototype of the > library > > >> and advice on how to move all this forward, from a prototype stage to > > >> production. For further reference, see: > > >> > > >>https://lists.freedesktop.org/archives/mesa-dev/2017- > November/177632.html > > >> > > >> From the thread above, we came up with very interesting high level > > >> design ideas for one of the currently missing parts in the library: > > >> Usage transitions. That's something I'll personally work on during the > > >> following weeks. > > >> > > >> > > >> In the meantime, I've been working on putting together an open source > > >> implementation of the allocator mechanisms using the Nouveau driver > for > > >> all to be able to play with. > > >> > > >> Below I'm seeking feedback on a bunch of changes I had to make to > > >> different components of the graphics stack: > > >> > > >> ** Allocator ** > > >> > > >> An allocator driver implementation on top of Nouveau. The current > > >> implementation only handles pitch linear layouts, but that's enough > > >> to have the kmscube port working using the allocator and Nouveau > > >> drivers. > > >> > > >> You can pull these changes from > > >> > > >> https://github.com/mvicomoya/allocator/tree/wip/mvicomoya/ > nouveau-driver > > >> > > >> ** Mesa ** > > >> > > >> James's kmscube port to use the allocator relies on the > > >> EXT_external_objects extension to import allocator allocations to > > >> OpenGL as a texture object. However, the Nouveau implementation of > > >> these mechanisms is missing in Mesa, so I went ahead and added them. > > >> > > >> You can pull these changes from > > >> > > >> https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/EXT_ > external_objects-nouveau > > >> > > >> Also, James's kmscube port uses the NVX_unix_allocator_import > > >> extension to attach allocator metadata to texture objects so the > > >> driver knows how to deal with the imported memory. > > >> > > >> Note that there isn't a formal spec for this extension yet. For now, > > >> it just serves as an experimental mechanism to import allocator > > >> memory in OpenGL, and attach metadata to texture objects. > > >> > > >> You can pull these changes (written on top of the above) from: > > >> > > >> https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/NVX_ > unix_allocator_import > > >> > > >> ** kmscube ** > > >> > > >> Mostly minor fixes and improvements on top of James's port to use > the > > >> allocator. Main thing is the allocator initialization path will use > > >> EGL_MESA_platform_surfaceless if EGLDevice platform isn't supported > > >> by the underlying EGL implementation. > > >> > > >> You can pull these changes from: > > >> > > >> https://github.com/mvicomoya/kmscube/tree/wip/mvicomoya/ > allocator-nouveau > > >> > > >> > > >> With all the above you should be able to get kmscube working using the > > >> allocator on top of the Nouveau driver. > > >> > > >> > > >> Another of the missing pieces before we can move this to production is > > >> importing allocations to DRM FB objects. This is probably one of the > > >> most sensitive parts of the project as it requires > modification/addition > > >> of kernel driver interfaces. > > >> > > >> At XDC2017, James had several hallway conversations with several > people > > >> about this, all having different opinions. I'd like to take this > > >> opportunity to also start a discussion about what's the best option to > > >> create a path to get allocator allocations added as DRM FB objects. > > >> > > >> These are the few options we've considered to start with: > > >> > > >> A) Have vendor-private ioctls to set properties on GEM objects that > > >> are inherited by the FB objects. This is how our (NVIDIA) desktop > > >> DRM driver currently works. This would require every vendor to > add > > >> their own ioctl to process allocator metadata, but the metadata > is > > >> actually a vendor-agnostic object more like DRM modifiers. We'd > > >> like to come up with a vendor-agnostic solutions that can be > > >> integrated to core DRM. > > >> > > >> B) Add a new drmModeAddFBWithMetadata() command that takes allocator > > >> metadata blobs for each plane of the FB. Some people in the > > >> community have mentioned this is their preferred d
Re: [PATCH v2 1/8] drm/rockchip/dsi: correct Feedback divider setting
Nickey Yang writes:
> This patch correct Feedback divider setting:
> 1、Set Feedback divider [8:5] when HIGH_PROGRAM_EN
> 2、Due to the use of a "by 2 pre-scaler," the range of the
> feedback multiplication Feedback divider is limited to even
> division numbers, and Feedback divider must be greater than
> 12, less than 1000.
> 3、Make the previously configured Feedback divider(LSB)
> factors effective
> 4、Add the definition of the MIPI PHY register.
There's too much going on in this patch. A good rule of thumb is that
if you have enumerate the changes in a list like above, you should
probably break it down it that many patches instead. Any of these
changes could be a regression and when we bisect to this commit, we'll
have split it by hand to see which of the four independent changes is
causing trouble.
As a minimum, the addition of the register #defines should be a separate
patch. As it is, it makes it really hard to review the functional
changes...
> Signed-off-by: Nickey Yang
> ---
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 219
> ++---
> 1 file changed, 146 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 9a20b9d..c933a3a 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -228,7 +228,7 @@
> #define LOW_PROGRAM_EN 0
> #define HIGH_PROGRAM_EN BIT(7)
> #define LOOP_DIV_LOW_SEL(val)(((val) - 1) & 0x1f)
> -#define LOOP_DIV_HIGH_SEL(val) val) - 1) >> 5) & 0x1f)
> +#define LOOP_DIV_HIGH_SEL(val) val) - 1) >> 5) & 0xf)
> #define PLL_LOOP_DIV_EN BIT(5)
> #define PLL_INPUT_DIV_EN BIT(4)
>
> @@ -254,6 +254,28 @@
> #define DW_MIPI_NEEDS_PHY_CFG_CLKBIT(0)
> #define DW_MIPI_NEEDS_GRF_CLKBIT(1)
>
> +#define PLL_BIAS_CUR_SEL_CAP_VCO_CONTROL 0x10
> +#define PLL_CP_CONTROL_PLL_LOCK_BYPASS 0x11
> +#define PLL_LPF_AND_CP_CONTROL 0x12
> +#define PLL_INPUT_DIVIDER_RATIO 0x17
> +#define PLL_LOOP_DIVIDER_RATIO 0x18
> +#define PLL_INPUT_AND_LOOP_DIVIDER_RATIOS_CONTROL 0x19
> +#define BANDGAP_AND_BIAS_CONTROL 0x20
> +#define TERMINATION_RESISTER_CONTROL 0x21
> +#define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY 0x22
> +#define HS_RX_CONTROL_OF_LANE_0 0x44
> +#define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL 0x60
> +#define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL 0x61
> +#define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL 0x62
> +#define HS_TX_CLOCK_LANE_TRAIL_STATE_TIME_CONTROL 0x63
> +#define HS_TX_CLOCK_LANE_EXIT_STATE_TIME_CONTROL 0x64
> +#define HS_TX_CLOCK_LANE_POST_TIME_CONTROL 0x65
> +#define HS_TX_DATA_LANE_REQUEST_STATE_TIME_CONTROL 0x70
> +#define HS_TX_DATA_LANE_PREPARE_STATE_TIME_CONTROL 0x71
> +#define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL 0x72
> +#define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL 0x73
> +#define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL 0x74
> +
> enum {
> BANDGAP_97_07,
> BANDGAP_98_05,
> @@ -447,53 +469,79 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
> return ret;
> }
>
> - dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE |
> - VCO_RANGE_CON_SEL(vco) |
> - VCO_IN_CAP_CON_LOW |
> - REF_BIAS_CUR_SEL);
> -
> - dw_mipi_dsi_phy_write(dsi, 0x11, CP_CURRENT_3MA);
> - dw_mipi_dsi_phy_write(dsi, 0x12, CP_PROGRAM_EN | LPF_PROGRAM_EN |
> - LPF_RESISTORS_20_KOHM);
> -
> - dw_mipi_dsi_phy_write(dsi, 0x44, HSFREQRANGE_SEL(testdin));
> -
> - dw_mipi_dsi_phy_write(dsi, 0x17, INPUT_DIVIDER(dsi->input_div));
> - dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_LOW_SEL(dsi->feedback_div) |
> - LOW_PROGRAM_EN);
> - dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_HIGH_SEL(dsi->feedback_div) |
> - HIGH_PROGRAM_EN);
> - dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN);
> -
> - dw_mipi_dsi_phy_write(dsi, 0x22, LOW_PROGRAM_EN |
> - BIASEXTR_SEL(BIASEXTR_127_7));
> - dw_mipi_dsi_phy_write(dsi, 0x22, HIGH_PROGRAM_EN |
> - BANDGAP_SEL(BANDGAP_96_10));
> -
> - dw_mipi_dsi_phy_write(dsi, 0x20, POWER_CONTROL | INTERNAL_REG_CURRENT |
> - BIAS_BLOCK_ON | BANDGAP_ON);
> -
> - dw_mipi_dsi_phy_write(dsi, 0x21, TER_RESISTOR_LOW | TER_CAL_DONE |
> - SETRD_MAX | TER_RESISTORS_ON);
> - dw_mipi_dsi_phy_write(dsi, 0x21, TER_RESISTOR_HIGH | LEVEL_SHIFTERS_ON |
> - SETRD_MAX | POWER_MANAGE |
> - TER_RESISTORS_ON);
> -
> - dw_mipi_dsi_phy_write(dsi, 0x60, TLP_PROGRAM_EN | ns2bc(dsi, 500));
> - dw_mipi_dsi_ph
[PATCH] drm_fourcc: Document linear modifier
On Wed, Nov 9, 2016 at 4:36 AM, Daniel Vetter
wrote:
> Not setting the fb modifiers flag is something different from setting
> the fb modifiers to 0 (which means explicitly linear). We kinda failed
> to document that properly. Spotted by Kristian.
>
> Cc: hoegsberg at google.com
> Signed-off-by: Daniel Vetter
>
Thanks, looks good.
Reviewed-by: Kristian H. Kristensen
> ---
> include/uapi/drm/drm_fourcc.h | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index da49f69e4d7e..2fca7e1f6aab 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -172,6 +172,16 @@ extern "C" {
> * authoritative source for all of these.
> */
>
> +/*
> + * Linear Layout
> + *
> + * Just plain linear layout. Note that this is different from no
> specifying any
> + * modifier (e.g. not setting DRM_MODE_FB_MODIFIERS in the DRM_ADDFB2
> ioctl),
> + * which tells the driver to also take driver-internal information into
> account
> + * and so might actually result in a tiled framebuffer.
> + */
> +#define DRM_FORMAT_MOD_LINEAR fourcc_mod_code(NONE, 0)
> +
> /* Intel framebuffer modifiers */
>
> /*
> --
> 2.7.4
>
>
-- next part --
An HTML attachment was scrubbed...
URL:
