Re: [PATCH 49/89] drm/i915/skl: Program the DDB allocation

Ville Syrjälä
2014-09-19T10:03:19Z

On Thu, Sep 04, 2014 at 12:27:15PM +0100, Damien Lespiau wrote:
> v2: Adapt to the planes/cursor split
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 16 ++++++++++++++++
> drivers/gpu/drm/i915/intel_pm.c | 9 +++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9fbce2c..414c2a5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4602,6 +4602,8 @@ enum punit_power_well {
> #define _PLANE_KEYMSK_2_A 0x70298
> #define _PLANE_KEYMAX_1_A 0x701a0
> #define _PLANE_KEYMAX_2_A 0x702a0
> +#define _PLANE_BUF_CFG_1_A 0x7027c
> +#define _PLANE_BUF_CFG_2_A 0x7037c
>
> #define _PLANE_CTL_1_B 0x71180
> #define _PLANE_CTL_2_B 0x71280
> @@ -4679,6 +4681,20 @@ enum punit_power_well {
> #define PLANE_KEYMAX(pipe, plane) \
> _PLANE(plane, _PLANE_KEYMAX_1(pipe), _PLANE_KEYMAX_2(pipe))
>
> +#define _PLANE_BUF_CFG_1_B 0x7127c
> +#define _PLANE_BUF_CFG_2_B 0x7137c
> +#define _PLANE_BUF_CFG_1(pipe) \
> + _PIPE(pipe, _PLANE_BUF_CFG_1_A, _PLANE_BUF_CFG_1_B)
> +#define _PLANE_BUF_CFG_2(pipe) \
> + _PIPE(pipe, _PLANE_BUF_CFG_2_A, _PLANE_BUF_CFG_2_B)
> +#define PLANE_BUF_CFG(pipe, plane) \
> + _PLANE(plane, _PLANE_BUF_CFG_1(pipe), _PLANE_BUF_CFG_2(pipe))
> +
> +/* SKL new cursor registers */
> +#define _CUR_BUF_CFG_A 0x7017c
> +#define _CUR_BUF_CFG_B 0x7117c
> +#define CUR_BUF_CFG(pipe) _PIPE(pipe, _CUR_BUF_CFG_A, _CUR_BUF_CFG_B)
> +

More macros to confuse me :P

The magic numbers look correct again though.

> /* VBIOS regs */
> #define VGACNTRL 0x71400
> # define VGA_DISP_DISABLE (1 << 31)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0ddcbad..756ff16 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3393,6 +3393,15 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
> I915_WRITE(PLANE_WM_TRANS(pipe, i),
> new->plane_trans[pipe][i]);
> I915_WRITE(CUR_WM_TRANS(pipe), new->cursor_trans[pipe]);
> +
> + for (i = 0; i < intel_num_planes(crtc); i++)
> + I915_WRITE(PLANE_BUF_CFG(pipe, i),
> + new->ddb.plane[pipe][i].end << 16 |

The spec doesn't say if end is inclusive or exclusive. Someone actually
confirmed that from someone or tested it?

> + new->ddb.plane[pipe][i].start);
> +
> + I915_WRITE(CUR_BUF_CFG(pipe),
> + new->ddb.cursor[pipe].end << 16 |
> + new->ddb.cursor[pipe].start);
> }
> }

And here we land in deep doodoo wrt. changing the DDB allocation
for active pipes. Someone needs to implement the correct logic to
sequence the DDB allocation changes so that the allocations don't
overlap at any point in time.

So this needs a big FIXME at the very least.

>
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC