Re: [PATCH] drm/komeda: Adds VRR support
On Fri, Jul 05, 2019 at 09:23:20AM +0200, Daniel Vetter wrote:
> On Thu, Jul 4, 2019 at 5:41 PM Brian Starkey wrote:
> >
> > Hi,
> >
> > On Thu, Jul 04, 2019 at 11:57:00AM +0100, james qian wang (Arm Technology
> > China) wrote:
> > > On Wed, Jul 03, 2019 at 12:01:49PM +0200, Daniel Vetter wrote:
> > > >
> > > > Uh, what exactly are you doing reinventing uapi properties that we
> > > > already
> > > > standardized?
> > > >
> > >
> > > Sorry, Will use the mode_config->VRR_ENABLED
> >
> > Let's have a chat about what you're planning here. The upstream VRR
> > properties aren't a direct match for our HW (which we discussed
> > before) - so either we need to hide that in the kernel with some frame
> > timing heuristics, or we shouldn't expose our feature via the existing
> > properties.
> >
> > IMO, it's better for Komeda to just allow setting a new CRTC mode to
> > one with a different VFP (but everything else the same) without a full
> > modeset.
> >
> > You could try and implement the upstream VRR properties too - but you
> > can get the functionality added by this patch without changing any
> > UAPI.
> >
> > (Note the only reason we ever added the idea of passing in VFP by
> > itself is because in ADF, modeset was a separate ioctl entirely, so we
> > couldn't do it atomically)
>
> If you want to see an example of how to do changes in the display mode
> (like refresh rate, I have no idea what you mean with VFP, just
> guessing) look at i915. We clear drm_crtc_state->mode_changed if it's
> a mode change we can handle without a full modeset. That gives you
> userspace-controlled variable refresh rate.
>
> The VRR properties are for true VRR, i.e. the hw (with or without help
> of the kernel) decides how long to make each vblank for every frame
> individually, within certain limitats set by the monitor in its EDID
> (or for panels maybe in DT).
>
> > > we use this private property because we're switching to in-tree, before
> > > finish the switch, we still need to maintain our out-of-tree driver which
> > > depend on a older and doesn't have the VRR_ENABLED property. for avoid
> > > diverging the two branch. my old plan is first switch to in-tree, then
> > > drop
> > > the out-of-tree driver and then unify the usage.
> > >
> > > > > + if (!prop)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + drm_object_attach_property(>base, prop, 0);
> > > > > + kcrtc->vrr_enable_property = prop;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > static struct drm_plane *
> > > > > get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc
> > > > > *crtc)
> > > > > {
> > > > > @@ -659,6 +717,10 @@ static int komeda_crtc_add(struct komeda_kms_dev
> > > > > *kms,
> > > > > if (err)
> > > > > return err;
> > > > >
> > > > > + err = komeda_crtc_create_vrr_property(kcrtc);
> > > > > + if (err)
> > > > > + return err;
> > > > > +
> > > > > return err;
> > > > > }
> > > > >
> > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > > index dc1d436..d0cf838 100644
> > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > > @@ -98,6 +98,12 @@ struct komeda_crtc {
> > > > >
> > > > > /** @slave_planes_property: property for slaves of the planes */
> > > > > struct drm_property *slave_planes_property;
> > > >
> > > > And this seems to not be the first time this happened. Looking at komeda
> > > > with a quick git grep on properties you've actually accumulated quite a
> > > > pile of such driver properties already. Where's the userspace for this?
> > > > Where's the uapi discussions for this stuff? Where's the igt tests for
> > > > this (yes a bunch are after we agreed to have testcases for this).
> > > >
> > > > I know that in the past we've been somewhat sloppy properties, but that
> > > > was a mistake and we've cranked down on this hard. Probably need to fix
> > > > this with a pile of reverts and start over.
> > > > -Daniel
> > >
> > > Sorry again.
> > >
> > > First I'll send some patches to remove these private properties.
> > >
> > > and then discuss for how to impelement them.
> > >
> > > The current komeda privates are:
> > >
> > > crtc:
> > >clock_ratio
> > >slave_planes
> > >
> > > plane:
> > >img_enhancement
> > >layer_split
> > >
> > > Layer_split: it can be deleted and computed in kernel.
> > >
> > > img_enhancement:
> > > it is for image enhancement, can be removed and computed in kernel.
> > > but I'd like to have it, since it's a seperated function (NOT only
> > > for scaling or YUV format), I think only user can real know if need
> > > to enable it.
> > >
> > >
> > > img_enhancement:
> > > it is for image enhancement, can be removed and computed in kernel.
> > > but I'd like to have it, since it's a seperated function (NOT only
> > > for scaling or YUV
Re: [PATCH] drm/komeda: Adds VRR support
On Thu, Jul 04, 2019 at 11:41:38PM +0800, Brian Starkey wrote:
> Hi,
>
> On Thu, Jul 04, 2019 at 11:57:00AM +0100, james qian wang (Arm Technology
> China) wrote:
> > On Wed, Jul 03, 2019 at 12:01:49PM +0200, Daniel Vetter wrote:
> > >
> > > Uh, what exactly are you doing reinventing uapi properties that we already
> > > standardized?
> > >
> >
> > Sorry, Will use the mode_config->VRR_ENABLED
>
> Let's have a chat about what you're planning here. The upstream VRR
> properties aren't a direct match for our HW (which we discussed
> before) - so either we need to hide that in the kernel with some frame
> timing heuristics, or we shouldn't expose our feature via the existing
> properties.
@Brian:
/**
* @vrr_enabled:
*
* Indicates if variable refresh rate should be enabled for the CRTC.
* Support for the requested vrr state will depend on driver and
* hardware capabiltiy - lacking support is not treated as failure.
*/
bool vrr_enabled;
It's not HW specific flag (like AMD freesync), I think can use this standard
flag.
> IMO, it's better for Komeda to just allow setting a new CRTC mode to
> one with a different VFP (but everything else the same) without a full
> modeset.
>
> You could try and implement the upstream VRR properties too - but you
> can get the functionality added by this patch without changing any
> UAPI.
>
> (Note the only reason we ever added the idea of passing in VFP by
> itself is because in ADF, modeset was a separate ioctl entirely, so we
> couldn't do it atomically)
Yes, we can.
But DRM-KMS (the helpers) default doesn't support such light mode-set.
we can not rely on the helpers, but need to implement by ourselves.
Is it worth to do it?
My plan is:
First:
I think the key problem here is not how to enable VRR for our display,
but how to pass the VRR caps from the connector to our display.
And it's not only the VRR prblem but like the command_mode/dual-link
all the none standard connector features that needed by our HW.
Unlike the intel/amd/NV mostly they have their own transmitter HW
(and connector driver), So they can easily pass the info between
connector and display.
But for us that's the third part, and we can not do any assumption to
them, and for us they are the drm_connector, but we can not got these
infos via a drm_connector.
So I think we need a standard way to pass these private infos.
I plan to add a new query function to drm_bridge like
(*query)(u64 query_id, xxx-type return_va);
and query_id is like the modifiers: first 8bit is vendor_id.
when the third part connector integrats to our display, if the
transmitter HW support the features that our display needed, they
can supply this query function to notify the caps support to us.
If the connector doesn't have this query, or query fail we treat
the feature is not support by this connector.
And For this VRR.
User only need to En/Dis VRR, Once komeda received Enable command
- query the connector for VFP range
1. connector doesn't support VRR, ignore or return error ?.
2. got the VFP range, configure the display according the range.
Once the disable received, restore the mode vfp.
Thanks
James
> >
> > we use this private property because we're switching to in-tree, before
> > finish the switch, we still need to maintain our out-of-tree driver which
> > depend on a older and doesn't have the VRR_ENABLED property. for avoid
> > diverging the two branch. my old plan is first switch to in-tree, then drop
> > the out-of-tree driver and then unify the usage.
> >
> > > > + if (!prop)
> > > > + return -ENOMEM;
> > > > +
> > > > + drm_object_attach_property(>base, prop, 0);
> > > > + kcrtc->vrr_enable_property = prop;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > static struct drm_plane *
> > > > get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
> > > > {
> > > > @@ -659,6 +717,10 @@ static int komeda_crtc_add(struct komeda_kms_dev
> > > > *kms,
> > > > if (err)
> > > > return err;
> > > >
> > > > + err = komeda_crtc_create_vrr_property(kcrtc);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > return err;
> > > > }
> > > >
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > index dc1d436..d0cf838 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > @@ -98,6 +98,12 @@ struct komeda_crtc {
> > > >
> > > > /** @slave_planes_property: property for slaves of the planes */
> > > > struct drm_property *slave_planes_property;
> > >
> > > And this seems to not be the first time this happened. Looking at komeda
> > > with a quick git grep on properties you've actually accumulated quite a
> > > pile of such driver
Re: [PATCH] drm/komeda: Adds VRR support
On Thu, Jul 4, 2019 at 5:41 PM Brian Starkey wrote:
>
> Hi,
>
> On Thu, Jul 04, 2019 at 11:57:00AM +0100, james qian wang (Arm Technology
> China) wrote:
> > On Wed, Jul 03, 2019 at 12:01:49PM +0200, Daniel Vetter wrote:
> > >
> > > Uh, what exactly are you doing reinventing uapi properties that we already
> > > standardized?
> > >
> >
> > Sorry, Will use the mode_config->VRR_ENABLED
>
> Let's have a chat about what you're planning here. The upstream VRR
> properties aren't a direct match for our HW (which we discussed
> before) - so either we need to hide that in the kernel with some frame
> timing heuristics, or we shouldn't expose our feature via the existing
> properties.
>
> IMO, it's better for Komeda to just allow setting a new CRTC mode to
> one with a different VFP (but everything else the same) without a full
> modeset.
>
> You could try and implement the upstream VRR properties too - but you
> can get the functionality added by this patch without changing any
> UAPI.
>
> (Note the only reason we ever added the idea of passing in VFP by
> itself is because in ADF, modeset was a separate ioctl entirely, so we
> couldn't do it atomically)
If you want to see an example of how to do changes in the display mode
(like refresh rate, I have no idea what you mean with VFP, just
guessing) look at i915. We clear drm_crtc_state->mode_changed if it's
a mode change we can handle without a full modeset. That gives you
userspace-controlled variable refresh rate.
The VRR properties are for true VRR, i.e. the hw (with or without help
of the kernel) decides how long to make each vblank for every frame
individually, within certain limitats set by the monitor in its EDID
(or for panels maybe in DT).
> > we use this private property because we're switching to in-tree, before
> > finish the switch, we still need to maintain our out-of-tree driver which
> > depend on a older and doesn't have the VRR_ENABLED property. for avoid
> > diverging the two branch. my old plan is first switch to in-tree, then drop
> > the out-of-tree driver and then unify the usage.
> >
> > > > + if (!prop)
> > > > + return -ENOMEM;
> > > > +
> > > > + drm_object_attach_property(>base, prop, 0);
> > > > + kcrtc->vrr_enable_property = prop;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > static struct drm_plane *
> > > > get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
> > > > {
> > > > @@ -659,6 +717,10 @@ static int komeda_crtc_add(struct komeda_kms_dev
> > > > *kms,
> > > > if (err)
> > > > return err;
> > > >
> > > > + err = komeda_crtc_create_vrr_property(kcrtc);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > return err;
> > > > }
> > > >
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > index dc1d436..d0cf838 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > > @@ -98,6 +98,12 @@ struct komeda_crtc {
> > > >
> > > > /** @slave_planes_property: property for slaves of the planes */
> > > > struct drm_property *slave_planes_property;
> > >
> > > And this seems to not be the first time this happened. Looking at komeda
> > > with a quick git grep on properties you've actually accumulated quite a
> > > pile of such driver properties already. Where's the userspace for this?
> > > Where's the uapi discussions for this stuff? Where's the igt tests for
> > > this (yes a bunch are after we agreed to have testcases for this).
> > >
> > > I know that in the past we've been somewhat sloppy properties, but that
> > > was a mistake and we've cranked down on this hard. Probably need to fix
> > > this with a pile of reverts and start over.
> > > -Daniel
> >
> > Sorry again.
> >
> > First I'll send some patches to remove these private properties.
> >
> > and then discuss for how to impelement them.
> >
> > The current komeda privates are:
> >
> > crtc:
> >clock_ratio
> >slave_planes
> >
> > plane:
> >img_enhancement
> >layer_split
> >
> > Layer_split: it can be deleted and computed in kernel.
> >
> > img_enhancement:
> > it is for image enhancement, can be removed and computed in kernel.
> > but I'd like to have it, since it's a seperated function (NOT only
> > for scaling or YUV format), I think only user can real know if need
> > to enable it.
> >
> >
> > img_enhancement:
> > it is for image enhancement, can be removed and computed in kernel.
> > but I'd like to have it, since it's a seperated function (NOT only
> > for scaling or YUV format), I think only user can real know if need
> > to enable it.
> > I think maybe we can add it CORE as an optional drm_plane property.
>
> I really don't think we should be exposing this. It's purely there to
> help improve an image after scaling (effectively, sharpening). It's
> not a general purpose "image enhancer". Exposing a
Re: [PATCH] drm/komeda: Adds VRR support
Hi,
On Thu, Jul 04, 2019 at 11:57:00AM +0100, james qian wang (Arm Technology
China) wrote:
> On Wed, Jul 03, 2019 at 12:01:49PM +0200, Daniel Vetter wrote:
> >
> > Uh, what exactly are you doing reinventing uapi properties that we already
> > standardized?
> >
>
> Sorry, Will use the mode_config->VRR_ENABLED
Let's have a chat about what you're planning here. The upstream VRR
properties aren't a direct match for our HW (which we discussed
before) - so either we need to hide that in the kernel with some frame
timing heuristics, or we shouldn't expose our feature via the existing
properties.
IMO, it's better for Komeda to just allow setting a new CRTC mode to
one with a different VFP (but everything else the same) without a full
modeset.
You could try and implement the upstream VRR properties too - but you
can get the functionality added by this patch without changing any
UAPI.
(Note the only reason we ever added the idea of passing in VFP by
itself is because in ADF, modeset was a separate ioctl entirely, so we
couldn't do it atomically)
>
> we use this private property because we're switching to in-tree, before
> finish the switch, we still need to maintain our out-of-tree driver which
> depend on a older and doesn't have the VRR_ENABLED property. for avoid
> diverging the two branch. my old plan is first switch to in-tree, then drop
> the out-of-tree driver and then unify the usage.
>
> > > + if (!prop)
> > > + return -ENOMEM;
> > > +
> > > + drm_object_attach_property(>base, prop, 0);
> > > + kcrtc->vrr_enable_property = prop;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static struct drm_plane *
> > > get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
> > > {
> > > @@ -659,6 +717,10 @@ static int komeda_crtc_add(struct komeda_kms_dev
> > > *kms,
> > > if (err)
> > > return err;
> > >
> > > + err = komeda_crtc_create_vrr_property(kcrtc);
> > > + if (err)
> > > + return err;
> > > +
> > > return err;
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > index dc1d436..d0cf838 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > > @@ -98,6 +98,12 @@ struct komeda_crtc {
> > >
> > > /** @slave_planes_property: property for slaves of the planes */
> > > struct drm_property *slave_planes_property;
> >
> > And this seems to not be the first time this happened. Looking at komeda
> > with a quick git grep on properties you've actually accumulated quite a
> > pile of such driver properties already. Where's the userspace for this?
> > Where's the uapi discussions for this stuff? Where's the igt tests for
> > this (yes a bunch are after we agreed to have testcases for this).
> >
> > I know that in the past we've been somewhat sloppy properties, but that
> > was a mistake and we've cranked down on this hard. Probably need to fix
> > this with a pile of reverts and start over.
> > -Daniel
>
> Sorry again.
>
> First I'll send some patches to remove these private properties.
>
> and then discuss for how to impelement them.
>
> The current komeda privates are:
>
> crtc:
>clock_ratio
>slave_planes
>
> plane:
>img_enhancement
>layer_split
>
> Layer_split: it can be deleted and computed in kernel.
>
> img_enhancement:
> it is for image enhancement, can be removed and computed in kernel.
> but I'd like to have it, since it's a seperated function (NOT only
> for scaling or YUV format), I think only user can real know if need
> to enable it.
>
>
> img_enhancement:
> it is for image enhancement, can be removed and computed in kernel.
> but I'd like to have it, since it's a seperated function (NOT only
> for scaling or YUV format), I think only user can real know if need
> to enable it.
> I think maybe we can add it CORE as an optional drm_plane property.
I really don't think we should be exposing this. It's purely there to
help improve an image after scaling (effectively, sharpening). It's
not a general purpose "image enhancer". Exposing a property which says
"image enhancement" isn't useful to any application - what kind of
enhancement is it doing?
>
> clock_ratio:
> It's the clock ratio of (main engine lock/output pixel clk) for
> komeda HW's downscaling restriction, as below:
>
> D71 downscaling must satisfy the following equation
>
> MCLK h_in * v_in
> --- >= -
> PXLCLK (h_total - (1 + 2 * v_in / v_out)) * v_out
>
> In only horizontal downscaling situation, the right side should be
> multiplied by (h_total - 3) / (h_active - 3), then equation becomes
>
> MCLK h_in
> --- >=
> PXLCLK (h_active - 3)
>
> slave_planes:
> it's not only for the zpos, but most importantly for notify the user
> to group the
Re: [PATCH] drm/komeda: Adds VRR support
On Wed, Jul 03, 2019 at 12:01:49PM +0200, Daniel Vetter wrote:
> On Wed, Jul 03, 2019 at 07:26:16AM +, Lowry Li (Arm Technology China)
> wrote:
> > Adds a new drm property "vrr" and "vrr_enable" and implemented
> > the set/get functions, through which userspace could set vfp
> > data to komeda.
> >
> > Signed-off-by: Lowry Li (Arm Technology China)
> > ---
> > .../gpu/drm/arm/display/komeda/d71/d71_component.c | 6 +++
> > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 62
> > ++
> > drivers/gpu/drm/arm/display/komeda/komeda_kms.h| 12 +
> > .../gpu/drm/arm/display/komeda/komeda_pipeline.h | 4 +-
> > 4 files changed, 83 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index ed3f273..c1355f5 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -1065,6 +1065,7 @@ static void d71_timing_ctrlr_update(struct
> > komeda_component *c,
> > struct komeda_component_state *state)
> > {
> > struct drm_crtc_state *crtc_st = state->crtc->state;
> > +struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc_st);
> > struct drm_display_mode *mode = _st->adjusted_mode;
> > u32 __iomem *reg = c->reg;
> > u32 hactive, hfront_porch, hback_porch, hsync_len;
> > @@ -1102,6 +1103,9 @@ static void d71_timing_ctrlr_update(struct
> > komeda_component *c,
> > value |= BS_CTRL_DL;
> > }
> >
> > +if (kcrtc_st->en_vrr)
> > +malidp_write32_mask(reg, BS_VINTERVALS, 0x3FFF, kcrtc_st->vfp);
> > +
> > malidp_write32(reg, BLK_CONTROL, value);
> > }
> >
> > @@ -1171,6 +1175,8 @@ static int d71_timing_ctrlr_init(struct d71_dev *d71,
> > ctrlr = to_ctrlr(c);
> >
> > ctrlr->supports_dual_link = true;
> > +ctrlr->supports_vrr = true;
> > +set_range(>vfp_range, 0, 0x3FF);
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > index 4f580b0..3744e6d 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > @@ -467,6 +467,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
> >
> > state = kzalloc(sizeof(*state), GFP_KERNEL);
> > if (state) {
> > +state->vfp = 0;
> > +state->en_vrr = 0;
> > crtc->state = >base;
> > crtc->state->crtc = crtc;
> > }
> > @@ -487,6 +489,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
> > new->affected_pipes = old->active_pipes;
> > new->clock_ratio = old->clock_ratio;
> > new->max_slave_zorder = old->max_slave_zorder;
> > +new->vfp = old->vfp;
> > +new->en_vrr = old->en_vrr;
> >
> > return >base;
> > }
> > @@ -525,6 +529,30 @@ static void komeda_crtc_vblank_disable(struct drm_crtc
> > *crtc)
> >
> > if (property == kcrtc->clock_ratio_property) {
> > *val = kcrtc_st->clock_ratio;
> > +} else if (property == kcrtc->vrr_property) {
> > +*val = kcrtc_st->vfp;
> > +} else if (property == kcrtc->vrr_enable_property) {
> > +*val = kcrtc_st->en_vrr;
> > +} else {
> > +DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
> > +return -EINVAL;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int komeda_crtc_atomic_set_property(struct drm_crtc *crtc,
> > + struct drm_crtc_state *state,
> > + struct drm_property *property,
> > + uint64_t val)
> > +{
> > +struct komeda_crtc *kcrtc = to_kcrtc(crtc);
> > +struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(state);
> > +
> > +if (property == kcrtc->vrr_property) {
> > +kcrtc_st->vfp = val;
> > +} else if (property == kcrtc->vrr_enable_property) {
> > +kcrtc_st->en_vrr = val;
> > } else {
> > DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
> > return -EINVAL;
> > @@ -544,6 +572,7 @@ static void komeda_crtc_vblank_disable(struct drm_crtc
> > *crtc)
> > .enable_vblank= komeda_crtc_vblank_enable,
> > .disable_vblank= komeda_crtc_vblank_disable,
> > .atomic_get_property= komeda_crtc_atomic_get_property,
> > +.atomic_set_property= komeda_crtc_atomic_set_property,
> > };
> >
> > int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> > @@ -613,6 +642,35 @@ static int
> > komeda_crtc_create_slave_planes_property(struct komeda_crtc *kcrtc)
> > return 0;
> > }
> >
> > +static int komeda_crtc_create_vrr_property(struct komeda_crtc *kcrtc)
> > +{
> > +struct drm_crtc *crtc = >base;
> > +struct drm_property *prop;
> > +struct komeda_timing_ctrlr *ctrlr = kcrtc->master->ctrlr;
> > +
> > +if (!ctrlr->supports_vrr)
> > +return 0;
> > +
> > +prop = drm_property_create_range(crtc->dev, DRM_MODE_PROP_ATOMIC, "vrr",
> > + ctrlr->vfp_range.start,
> > + ctrlr->vfp_range.end);
> > +if (!prop)
> > +return -ENOMEM;
> > +
> > +drm_object_attach_property(>base, prop, 0);
> > +kcrtc->vrr_property = prop;
> > +
> > +prop = drm_property_create_bool(crtc->dev, DRM_MODE_PROP_ATOMIC,
> > +"enable_vrr");
>
> Uh, what
Re: [PATCH] drm/komeda: Adds VRR support
On Wed, Jul 03, 2019 at 07:26:16AM +, Lowry Li (Arm Technology China) wrote:
> Adds a new drm property "vrr" and "vrr_enable" and implemented
> the set/get functions, through which userspace could set vfp
> data to komeda.
>
> Signed-off-by: Lowry Li (Arm Technology China)
> ---
> .../gpu/drm/arm/display/komeda/d71/d71_component.c | 6 +++
> drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 62
> ++
> drivers/gpu/drm/arm/display/komeda/komeda_kms.h| 12 +
> .../gpu/drm/arm/display/komeda/komeda_pipeline.h | 4 +-
> 4 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index ed3f273..c1355f5 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -1065,6 +1065,7 @@ static void d71_timing_ctrlr_update(struct
> komeda_component *c,
> struct komeda_component_state *state)
> {
> struct drm_crtc_state *crtc_st = state->crtc->state;
> + struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc_st);
> struct drm_display_mode *mode = _st->adjusted_mode;
> u32 __iomem *reg = c->reg;
> u32 hactive, hfront_porch, hback_porch, hsync_len;
> @@ -1102,6 +1103,9 @@ static void d71_timing_ctrlr_update(struct
> komeda_component *c,
> value |= BS_CTRL_DL;
> }
>
> + if (kcrtc_st->en_vrr)
> + malidp_write32_mask(reg, BS_VINTERVALS, 0x3FFF, kcrtc_st->vfp);
> +
> malidp_write32(reg, BLK_CONTROL, value);
> }
>
> @@ -1171,6 +1175,8 @@ static int d71_timing_ctrlr_init(struct d71_dev *d71,
> ctrlr = to_ctrlr(c);
>
> ctrlr->supports_dual_link = true;
> + ctrlr->supports_vrr = true;
> + set_range(>vfp_range, 0, 0x3FF);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 4f580b0..3744e6d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -467,6 +467,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
>
> state = kzalloc(sizeof(*state), GFP_KERNEL);
> if (state) {
> + state->vfp = 0;
> + state->en_vrr = 0;
> crtc->state = >base;
> crtc->state->crtc = crtc;
> }
> @@ -487,6 +489,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
> new->affected_pipes = old->active_pipes;
> new->clock_ratio = old->clock_ratio;
> new->max_slave_zorder = old->max_slave_zorder;
> + new->vfp = old->vfp;
> + new->en_vrr = old->en_vrr;
>
> return >base;
> }
> @@ -525,6 +529,30 @@ static void komeda_crtc_vblank_disable(struct drm_crtc
> *crtc)
>
> if (property == kcrtc->clock_ratio_property) {
> *val = kcrtc_st->clock_ratio;
> + } else if (property == kcrtc->vrr_property) {
> + *val = kcrtc_st->vfp;
> + } else if (property == kcrtc->vrr_enable_property) {
> + *val = kcrtc_st->en_vrr;
> + } else {
> + DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int komeda_crtc_atomic_set_property(struct drm_crtc *crtc,
> +struct drm_crtc_state *state,
> +struct drm_property *property,
> +uint64_t val)
> +{
> + struct komeda_crtc *kcrtc = to_kcrtc(crtc);
> + struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(state);
> +
> + if (property == kcrtc->vrr_property) {
> + kcrtc_st->vfp = val;
> + } else if (property == kcrtc->vrr_enable_property) {
> + kcrtc_st->en_vrr = val;
> } else {
> DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
> return -EINVAL;
> @@ -544,6 +572,7 @@ static void komeda_crtc_vblank_disable(struct drm_crtc
> *crtc)
> .enable_vblank = komeda_crtc_vblank_enable,
> .disable_vblank = komeda_crtc_vblank_disable,
> .atomic_get_property= komeda_crtc_atomic_get_property,
> + .atomic_set_property= komeda_crtc_atomic_set_property,
> };
>
> int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> @@ -613,6 +642,35 @@ static int
> komeda_crtc_create_slave_planes_property(struct komeda_crtc *kcrtc)
> return 0;
> }
>
> +static int komeda_crtc_create_vrr_property(struct komeda_crtc *kcrtc)
> +{
> + struct drm_crtc *crtc = >base;
> + struct drm_property *prop;
> + struct komeda_timing_ctrlr *ctrlr = kcrtc->master->ctrlr;
> +
> + if (!ctrlr->supports_vrr)
> + return 0;
> +
> + prop = drm_property_create_range(crtc->dev, DRM_MODE_PROP_ATOMIC, "vrr",
> +
[PATCH] drm/komeda: Adds VRR support
Adds a new drm property "vrr" and "vrr_enable" and implemented
the set/get functions, through which userspace could set vfp
data to komeda.
Signed-off-by: Lowry Li (Arm Technology China)
---
.../gpu/drm/arm/display/komeda/d71/d71_component.c | 6 +++
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 62 ++
drivers/gpu/drm/arm/display/komeda/komeda_kms.h| 12 +
.../gpu/drm/arm/display/komeda/komeda_pipeline.h | 4 +-
4 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index ed3f273..c1355f5 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -1065,6 +1065,7 @@ static void d71_timing_ctrlr_update(struct
komeda_component *c,
struct komeda_component_state *state)
{
struct drm_crtc_state *crtc_st = state->crtc->state;
+ struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc_st);
struct drm_display_mode *mode = _st->adjusted_mode;
u32 __iomem *reg = c->reg;
u32 hactive, hfront_porch, hback_porch, hsync_len;
@@ -1102,6 +1103,9 @@ static void d71_timing_ctrlr_update(struct
komeda_component *c,
value |= BS_CTRL_DL;
}
+ if (kcrtc_st->en_vrr)
+ malidp_write32_mask(reg, BS_VINTERVALS, 0x3FFF, kcrtc_st->vfp);
+
malidp_write32(reg, BLK_CONTROL, value);
}
@@ -1171,6 +1175,8 @@ static int d71_timing_ctrlr_init(struct d71_dev *d71,
ctrlr = to_ctrlr(c);
ctrlr->supports_dual_link = true;
+ ctrlr->supports_vrr = true;
+ set_range(>vfp_range, 0, 0x3FF);
return 0;
}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 4f580b0..3744e6d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -467,6 +467,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (state) {
+ state->vfp = 0;
+ state->en_vrr = 0;
crtc->state = >base;
crtc->state->crtc = crtc;
}
@@ -487,6 +489,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
new->affected_pipes = old->active_pipes;
new->clock_ratio = old->clock_ratio;
new->max_slave_zorder = old->max_slave_zorder;
+ new->vfp = old->vfp;
+ new->en_vrr = old->en_vrr;
return >base;
}
@@ -525,6 +529,30 @@ static void komeda_crtc_vblank_disable(struct drm_crtc
*crtc)
if (property == kcrtc->clock_ratio_property) {
*val = kcrtc_st->clock_ratio;
+ } else if (property == kcrtc->vrr_property) {
+ *val = kcrtc_st->vfp;
+ } else if (property == kcrtc->vrr_enable_property) {
+ *val = kcrtc_st->en_vrr;
+ } else {
+ DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int komeda_crtc_atomic_set_property(struct drm_crtc *crtc,
+ struct drm_crtc_state *state,
+ struct drm_property *property,
+ uint64_t val)
+{
+ struct komeda_crtc *kcrtc = to_kcrtc(crtc);
+ struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(state);
+
+ if (property == kcrtc->vrr_property) {
+ kcrtc_st->vfp = val;
+ } else if (property == kcrtc->vrr_enable_property) {
+ kcrtc_st->en_vrr = val;
} else {
DRM_DEBUG_DRIVER("Unknown property %s\n", property->name);
return -EINVAL;
@@ -544,6 +572,7 @@ static void komeda_crtc_vblank_disable(struct drm_crtc
*crtc)
.enable_vblank = komeda_crtc_vblank_enable,
.disable_vblank = komeda_crtc_vblank_disable,
.atomic_get_property= komeda_crtc_atomic_get_property,
+ .atomic_set_property= komeda_crtc_atomic_set_property,
};
int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
@@ -613,6 +642,35 @@ static int komeda_crtc_create_slave_planes_property(struct
komeda_crtc *kcrtc)
return 0;
}
+static int komeda_crtc_create_vrr_property(struct komeda_crtc *kcrtc)
+{
+ struct drm_crtc *crtc = >base;
+ struct drm_property *prop;
+ struct komeda_timing_ctrlr *ctrlr = kcrtc->master->ctrlr;
+
+ if (!ctrlr->supports_vrr)
+ return 0;
+
+ prop = drm_property_create_range(crtc->dev, DRM_MODE_PROP_ATOMIC, "vrr",
+ctrlr->vfp_range.start,
+ctrlr->vfp_range.end);
+ if (!prop)
+ return -ENOMEM;
+
+
