| Message ID | 20240424183820.3591593-5-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Link off between frames for edp | expand |
Hi Animesh, kernel test robot noticed the following build errors: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip drm/drm-next drm-exynos/exynos-drm-next next-20240426] [cannot apply to drm-intel/for-linux-next-fixes linus/master v6.9-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Animesh-Manna/drm-i915-alpm-Move-alpm-parameters-from-intel_psr/20240425-025652 base: git://anongit.freedesktop.org/drm-intel for-linux-next patch link: https://lore.kernel.org/r/20240424183820.3591593-5-animesh.manna%40intel.com patch subject: [PATCH v3 4/6] drm/i915/alpm: Add compute config for lobf config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20240427/202404270117.qRlKmdn7-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240427/202404270117.qRlKmdn7-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202404270117.qRlKmdn7-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/maps/map_funcs.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/hisi-spmi-controller.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/spmi-pmic-arb.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_cif.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_aec.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_netx.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_pruss.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_mf624.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pcmcia/pcmcia_rsrc.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pcmcia/yenta_socket.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pcmcia/i82092.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/corsair-cpro.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/mr75203.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/vhost/vringh.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/greybus/greybus.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/greybus/gb-es2.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/rpmsg/rpmsg_char.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/adc/ingenic-adc.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/adc/xilinx-ams.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/buffer/kfifo_buf.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-core.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-hub.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-aspeed.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-gpio.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-ast-cf.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-scom.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/siox/siox-bus-gpio.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/counter/ftm-quaddec.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/core/snd-pcm-dmaengine.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/core/sound_kunit.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/drivers/snd-pcmtest.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/pci/hda/snd-hda-cirrus-scodec-test.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/soc-topology-test.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-ab8500-codec.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-sigmadsp.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/codecs/snd-soc-wm-adsp.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/fsl/imx-pcm-dma.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-da7219.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-dmic.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-i2s-test.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-max98927.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-max98357a.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-max98373.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-nau8825.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-probe.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-rt274.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-rt286.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-rt298.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-rt5514.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-rt5663.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-rt5682.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/intel/avs/boards/snd-soc-avs-ssm4567.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/mxs/snd-soc-mxs-pcm.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/snd-soc-qcom-common.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/snd-soc-qcom-sdw.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/qcom/qdsp6/snd-q6dsp-common.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-intel-atom.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-acpi-intel-byt.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-acpi-intel-bdw.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-intel-hda-common.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-intel-hda-mlink.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-intel-hda.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-tng.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-skl.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-apl.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-cnl.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-icl.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-tgl.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-mtl.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/intel/snd-sof-pci-intel-lnl.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8m.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/snd-sof-imx8ulp.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/imx/imx-common.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mtk-adsp-common.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mt8195/snd-sof-mt8195.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/mediatek/mt8186/snd-sof-mt8186.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-utils.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-acpi.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-of.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/sof/snd-sof-pci.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/xilinx/snd-soc-xlnx-i2s.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/xilinx/snd-soc-xlnx-formatter-pcm.o WARNING: modpost: missing MODULE_DESCRIPTION() in sound/ac97_bus.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mtty.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mdpy.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mdpy-fb.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mbochs.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/configfs/configfs_sample.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/bytestream-example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/dma-example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/inttype-example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kfifo/record-example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kobject/kobject-example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kobject/kset-example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kprobes/kprobe_example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kprobes/kretprobe_example.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kmemleak/kmemleak-test.o WARNING: modpost: missing MODULE_DESCRIPTION() in samples/fprobe/fprobe_example.o >> ERROR: modpost: "intel_alpm_get_aux_less_status" [drivers/gpu/drm/xe/xe.ko] undefined! ERROR: modpost: "intel_alpm_compute_params" [drivers/gpu/drm/xe/xe.ko] undefined! >> ERROR: modpost: "intel_alpm_compute_lobf_config" [drivers/gpu/drm/xe/xe.ko] undefined! ERROR: modpost: "intel_alpm_configure" [drivers/gpu/drm/xe/xe.ko] undefined!
On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > Link Off Between Active Frames, is a new feature for eDP > that allows the panel to go to lower power state after > transmission of data. This is a feature on top of ALPM, AS SDP. > Add compute config during atomic-check phase. > > v1: RFC version. > v2: Add separate flag for auxless-alpm. [Jani] > v3: > - intel_dp->lobf_supported replaced with crtc_state->has_lobf. > [Jouni] > - Add DISPLAY_VER() check. [Jouni] > - Modify function name of get_aux_less_status. [Jani] > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_alpm.c | 48 > +++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_alpm.h | 5 ++ > .../drm/i915/display/intel_display_types.h | 4 ++ > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++ > 4 files changed, 61 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > b/drivers/gpu/drm/i915/display/intel_alpm.c > index 13bac3e8c8fa..3bb69ed16aab 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > @@ -11,6 +11,16 @@ > #include "intel_dp_aux.h" > #include "intel_psr_regs.h" > > +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp) > +{ > + u8 alpm_caps = 0; > + > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP, > + &alpm_caps) != 1) > + return false; > + return alpm_caps & DP_ALPM_AUX_LESS_CAP; > +} > + > /* > * See Bspec: 71632 for the table > * > @@ -242,6 +252,44 @@ bool intel_alpm_compute_params(struct intel_dp > *intel_dp, > return true; > } > > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > + struct intel_crtc_state > *crtc_state, > + struct drm_connector_state > *conn_state) > +{ > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + struct drm_display_mode *adjusted_mode = &crtc_state- > >hw.adjusted_mode; > + int waketime_in_lines, first_sdp_position; > + int context_latency, guardband; > + > + crtc_state->has_lobf = false; Drop this line. I think crtc_state is reset before doing this compute_config > + > + if (!intel_dp_is_edp(intel_dp)) > + return; > + > + if (DISPLAY_VER(i915) < 20) > + return; > + > + if (!intel_dp_as_sdp_supported(intel_dp)) > + return; > + > + if (crtc_state->has_psr) > + return; > + > + if (intel_alpm_compute_params(intel_dp, crtc_state)) { I think it is easier to read and helps avoiding big if blocks if you: if (!intel_alpm_compute_params(intel_dp, crtc_state()) return; This actually brings up another thing: do we want to spread intel_psr.c pollution by continue using these boolean return values? I would prefer changing intel_alpm_compute_params return value to "normal" int approach and return 0 on success. This would mean one more patch changing it. > + context_latency = adjusted_mode->crtc_vblank_start - > adjusted_mode->crtc_vdisplay; > + guardband = adjusted_mode->crtc_vtotal - > + adjusted_mode->crtc_vdisplay - > context_latency; > + first_sdp_position = adjusted_mode->crtc_vtotal - > adjusted_mode->crtc_vsync_start; > + if (intel_dp->alpm_parameters.auxless_alpm_supported) > + waketime_in_lines = intel_dp- > >alpm_parameters.io_wake_lines; > + else > + waketime_in_lines = intel_dp- > >alpm_parameters.fast_wake_lines; > + > + if ((context_latency + guardband) > > (first_sdp_position + waketime_in_lines)) > + crtc_state->has_lobf = true; crtc_state->has_lobf = (context_latency + guardband) > (first_sdp_position + waketime_in_lines); > + } > +} > + > static void lnl_alpm_configure(struct intel_dp *intel_dp) > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > b/drivers/gpu/drm/i915/display/intel_alpm.h > index c45d078e5a6b..b9602b71d28f 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > @@ -10,9 +10,14 @@ > > struct intel_dp; > struct intel_crtc_state; > +struct drm_connector_state; > > +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > struct intel_crtc_state *crtc_state); > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > + struct intel_crtc_state > *crtc_state, > + struct drm_connector_state > *conn_state); > void intel_alpm_configure(struct intel_dp *intel_dp); > > #endif > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index d94f50c6dc6c..5a0ffd5aa48b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1405,6 +1405,9 @@ struct intel_crtc_state { > > /* for loading single buffered registers during vblank */ > struct drm_vblank_work vblank_work; > + > + /* LOBF flag */ > + bool has_lobf; > }; > > enum intel_pipe_crc_source { > @@ -1835,6 +1838,7 @@ struct intel_dp { > u8 fast_wake_lines; > > /* LNL and beyond */ > + bool auxless_alpm_supported; > u8 check_entry_lines; > u8 silence_period_sym_clocks; > u8 lfps_half_cycle_num_of_syms; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index e05e25cd4a94..563739c6014c 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -48,6 +48,7 @@ > #include "i915_drv.h" > #include "i915_irq.h" > #include "i915_reg.h" > +#include "intel_alpm.h" > #include "intel_atomic.h" > #include "intel_audio.h" > #include "intel_backlight.h" > @@ -2997,6 +2998,7 @@ intel_dp_compute_config(struct intel_encoder > *encoder, > intel_vrr_compute_config(pipe_config, conn_state); > intel_dp_compute_as_sdp(intel_dp, pipe_config); > intel_psr_compute_config(intel_dp, pipe_config, conn_state); > + intel_alpm_compute_lobf_config(intel_dp, pipe_config, > conn_state); > intel_dp_drrs_compute_config(connector, pipe_config, > link_bpp_x16); > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, > pipe_config, conn_state); > @@ -6612,6 +6614,8 @@ static bool intel_edp_init_connector(struct > intel_dp *intel_dp, > > intel_pps_init_late(intel_dp); > > + intel_dp->alpm_parameters.auxless_alpm_supported = > intel_alpm_get_aux_less_status(intel_dp); How about aux_wake ? > + > return true; > > out_vdd_off:
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Friday, May 3, 2024 12:49 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com> > Subject: Re: [PATCH v3 4/6] drm/i915/alpm: Add compute config for lobf > > On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > > Link Off Between Active Frames, is a new feature for eDP > > that allows the panel to go to lower power state after > > transmission of data. This is a feature on top of ALPM, AS SDP. > > Add compute config during atomic-check phase. > > > > v1: RFC version. > > v2: Add separate flag for auxless-alpm. [Jani] > > v3: > > - intel_dp->lobf_supported replaced with crtc_state->has_lobf. > > [Jouni] > > - Add DISPLAY_VER() check. [Jouni] > > - Modify function name of get_aux_less_status. [Jani] > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_alpm.c | 48 > > +++++++++++++++++++ > > drivers/gpu/drm/i915/display/intel_alpm.h | 5 ++ > > .../drm/i915/display/intel_display_types.h | 4 ++ > > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++ > > 4 files changed, 61 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > index 13bac3e8c8fa..3bb69ed16aab 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > @@ -11,6 +11,16 @@ > > #include "intel_dp_aux.h" > > #include "intel_psr_regs.h" > > > > +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp) > > +{ > > + u8 alpm_caps = 0; > > + > > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP, > > + &alpm_caps) != 1) > > + return false; > > + return alpm_caps & DP_ALPM_AUX_LESS_CAP; > > +} > > + > > /* > > * See Bspec: 71632 for the table > > * > > @@ -242,6 +252,44 @@ bool intel_alpm_compute_params(struct intel_dp > > *intel_dp, > > return true; > > } > > > > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > > + struct intel_crtc_state > > *crtc_state, > > + struct drm_connector_state > > *conn_state) > > +{ > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > + struct drm_display_mode *adjusted_mode = &crtc_state- > > >hw.adjusted_mode; > > + int waketime_in_lines, first_sdp_position; > > + int context_latency, guardband; > > + > > + crtc_state->has_lobf = false; > > Drop this line. I think crtc_state is reset before doing this > compute_config Sure. > > > + > > + if (!intel_dp_is_edp(intel_dp)) > > + return; > > + > > + if (DISPLAY_VER(i915) < 20) > > + return; > > + > > + if (!intel_dp_as_sdp_supported(intel_dp)) > > + return; > > + > > + if (crtc_state->has_psr) > > + return; > > + > > + if (intel_alpm_compute_params(intel_dp, crtc_state)) { > > I think it is easier to read and helps avoiding big if blocks if you: > > if (!intel_alpm_compute_params(intel_dp, crtc_state()) > return; Ok. > > This actually brings up another thing: do we want to spread intel_psr.c > pollution by continue using these boolean return values? I would prefer > changing intel_alpm_compute_params return value to "normal" int > approach and return 0 on success. This would mean one more patch > changing it. Ok. > > > + context_latency = adjusted_mode->crtc_vblank_start - > > adjusted_mode->crtc_vdisplay; > > + guardband = adjusted_mode->crtc_vtotal - > > + adjusted_mode->crtc_vdisplay - > > context_latency; > > + first_sdp_position = adjusted_mode->crtc_vtotal - > > adjusted_mode->crtc_vsync_start; > > + if (intel_dp->alpm_parameters.auxless_alpm_supported) > > + waketime_in_lines = intel_dp- > > >alpm_parameters.io_wake_lines; > > + else > > + waketime_in_lines = intel_dp- > > >alpm_parameters.fast_wake_lines; > > + > > + if ((context_latency + guardband) > > > (first_sdp_position + waketime_in_lines)) > > + crtc_state->has_lobf = true; > > crtc_state->has_lobf = (context_latency + guardband) > > (first_sdp_position + waketime_in_lines); Ok. > > > + } > > +} > > + > > static void lnl_alpm_configure(struct intel_dp *intel_dp) > > { > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > index c45d078e5a6b..b9602b71d28f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > @@ -10,9 +10,14 @@ > > > > struct intel_dp; > > struct intel_crtc_state; > > +struct drm_connector_state; > > > > +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); > > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > > struct intel_crtc_state *crtc_state); > > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > > + struct intel_crtc_state > > *crtc_state, > > + struct drm_connector_state > > *conn_state); > > void intel_alpm_configure(struct intel_dp *intel_dp); > > > > #endif > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index d94f50c6dc6c..5a0ffd5aa48b 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1405,6 +1405,9 @@ struct intel_crtc_state { > > > > /* for loading single buffered registers during vblank */ > > struct drm_vblank_work vblank_work; > > + > > + /* LOBF flag */ > > + bool has_lobf; > > }; > > > > enum intel_pipe_crc_source { > > @@ -1835,6 +1838,7 @@ struct intel_dp { > > u8 fast_wake_lines; > > > > /* LNL and beyond */ > > + bool auxless_alpm_supported; > > u8 check_entry_lines; > > u8 silence_period_sym_clocks; > > u8 lfps_half_cycle_num_of_syms; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index e05e25cd4a94..563739c6014c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -48,6 +48,7 @@ > > #include "i915_drv.h" > > #include "i915_irq.h" > > #include "i915_reg.h" > > +#include "intel_alpm.h" > > #include "intel_atomic.h" > > #include "intel_audio.h" > > #include "intel_backlight.h" > > @@ -2997,6 +2998,7 @@ intel_dp_compute_config(struct intel_encoder > > *encoder, > > intel_vrr_compute_config(pipe_config, conn_state); > > intel_dp_compute_as_sdp(intel_dp, pipe_config); > > intel_psr_compute_config(intel_dp, pipe_config, conn_state); > > + intel_alpm_compute_lobf_config(intel_dp, pipe_config, > > conn_state); > > intel_dp_drrs_compute_config(connector, pipe_config, > > link_bpp_x16); > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, > > pipe_config, conn_state); > > @@ -6612,6 +6614,8 @@ static bool intel_edp_init_connector(struct > > intel_dp *intel_dp, > > > > intel_pps_init_late(intel_dp); > > > > + intel_dp->alpm_parameters.auxless_alpm_supported = > > intel_alpm_get_aux_less_status(intel_dp); > > How about aux_wake ? I felt auxless_alpm_supported will give both aux-less and aux-wake info, if it is true then aux-less otherwise aux-wake. Do you see any issue here? Regards, Animesh > > > + > > return true; > > > > out_vdd_off:
On Fri, 2024-05-03 at 08:42 +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Friday, May 3, 2024 12:49 PM > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R > > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com> > > Subject: Re: [PATCH v3 4/6] drm/i915/alpm: Add compute config for > > lobf > > > > On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > > > Link Off Between Active Frames, is a new feature for eDP > > > that allows the panel to go to lower power state after > > > transmission of data. This is a feature on top of ALPM, AS SDP. > > > Add compute config during atomic-check phase. > > > > > > v1: RFC version. > > > v2: Add separate flag for auxless-alpm. [Jani] > > > v3: > > > - intel_dp->lobf_supported replaced with crtc_state->has_lobf. > > > [Jouni] > > > - Add DISPLAY_VER() check. [Jouni] > > > - Modify function name of get_aux_less_status. [Jani] > > > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_alpm.c | 48 > > > +++++++++++++++++++ > > > drivers/gpu/drm/i915/display/intel_alpm.h | 5 ++ > > > .../drm/i915/display/intel_display_types.h | 4 ++ > > > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++ > > > 4 files changed, 61 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > > index 13bac3e8c8fa..3bb69ed16aab 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > > @@ -11,6 +11,16 @@ > > > #include "intel_dp_aux.h" > > > #include "intel_psr_regs.h" > > > > > > +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp) > > > +{ > > > + u8 alpm_caps = 0; > > > + > > > + if (drm_dp_dpcd_readb(&intel_dp->aux, > > > DP_RECEIVER_ALPM_CAP, > > > + &alpm_caps) != 1) > > > + return false; > > > + return alpm_caps & DP_ALPM_AUX_LESS_CAP; > > > +} > > > + > > > /* > > > * See Bspec: 71632 for the table > > > * > > > @@ -242,6 +252,44 @@ bool intel_alpm_compute_params(struct > > > intel_dp > > > *intel_dp, > > > return true; > > > } > > > > > > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > > > + struct intel_crtc_state > > > *crtc_state, > > > + struct drm_connector_state > > > *conn_state) > > > +{ > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > + struct drm_display_mode *adjusted_mode = &crtc_state- > > > > hw.adjusted_mode; > > > + int waketime_in_lines, first_sdp_position; > > > + int context_latency, guardband; > > > + > > > + crtc_state->has_lobf = false; > > > > Drop this line. I think crtc_state is reset before doing this > > compute_config > > Sure. > > > > > > + > > > + if (!intel_dp_is_edp(intel_dp)) > > > + return; > > > + > > > + if (DISPLAY_VER(i915) < 20) > > > + return; > > > + > > > + if (!intel_dp_as_sdp_supported(intel_dp)) > > > + return; > > > + > > > + if (crtc_state->has_psr) > > > + return; > > > + > > > + if (intel_alpm_compute_params(intel_dp, crtc_state)) { > > > > I think it is easier to read and helps avoiding big if blocks if > > you: > > > > if (!intel_alpm_compute_params(intel_dp, crtc_state()) > > return; > > Ok. > > > > > This actually brings up another thing: do we want to spread > > intel_psr.c > > pollution by continue using these boolean return values? I would > > prefer > > changing intel_alpm_compute_params return value to "normal" int > > approach and return 0 on success. This would mean one more patch > > changing it. > > Ok. > > > > > > + context_latency = adjusted_mode- > > > >crtc_vblank_start - > > > adjusted_mode->crtc_vdisplay; > > > + guardband = adjusted_mode->crtc_vtotal - > > > + adjusted_mode->crtc_vdisplay - > > > context_latency; > > > + first_sdp_position = adjusted_mode->crtc_vtotal - > > > adjusted_mode->crtc_vsync_start; > > > + if (intel_dp- > > > >alpm_parameters.auxless_alpm_supported) > > > + waketime_in_lines = intel_dp- > > > > alpm_parameters.io_wake_lines; > > > + else > > > + waketime_in_lines = intel_dp- > > > > alpm_parameters.fast_wake_lines; > > > + > > > + if ((context_latency + guardband) > > > > (first_sdp_position + waketime_in_lines)) > > > + crtc_state->has_lobf = true; > > > > crtc_state->has_lobf = (context_latency + guardband) > > > (first_sdp_position + waketime_in_lines); > > Ok. > > > > > > + } > > > +} > > > + > > > static void lnl_alpm_configure(struct intel_dp *intel_dp) > > > { > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > > index c45d078e5a6b..b9602b71d28f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > > @@ -10,9 +10,14 @@ > > > > > > struct intel_dp; > > > struct intel_crtc_state; > > > +struct drm_connector_state; > > > > > > +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); > > > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > > > struct intel_crtc_state > > > *crtc_state); > > > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > > > + struct intel_crtc_state > > > *crtc_state, > > > + struct drm_connector_state > > > *conn_state); > > > void intel_alpm_configure(struct intel_dp *intel_dp); > > > > > > #endif > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > > index d94f50c6dc6c..5a0ffd5aa48b 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > @@ -1405,6 +1405,9 @@ struct intel_crtc_state { > > > > > > /* for loading single buffered registers during vblank */ > > > struct drm_vblank_work vblank_work; > > > + > > > + /* LOBF flag */ > > > + bool has_lobf; > > > }; > > > > > > enum intel_pipe_crc_source { > > > @@ -1835,6 +1838,7 @@ struct intel_dp { > > > u8 fast_wake_lines; > > > > > > /* LNL and beyond */ > > > + bool auxless_alpm_supported; > > > u8 check_entry_lines; > > > u8 silence_period_sym_clocks; > > > u8 lfps_half_cycle_num_of_syms; > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index e05e25cd4a94..563739c6014c 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -48,6 +48,7 @@ > > > #include "i915_drv.h" > > > #include "i915_irq.h" > > > #include "i915_reg.h" > > > +#include "intel_alpm.h" > > > #include "intel_atomic.h" > > > #include "intel_audio.h" > > > #include "intel_backlight.h" > > > @@ -2997,6 +2998,7 @@ intel_dp_compute_config(struct > > > intel_encoder > > > *encoder, > > > intel_vrr_compute_config(pipe_config, conn_state); > > > intel_dp_compute_as_sdp(intel_dp, pipe_config); > > > intel_psr_compute_config(intel_dp, pipe_config, > > > conn_state); > > > + intel_alpm_compute_lobf_config(intel_dp, pipe_config, > > > conn_state); > > > intel_dp_drrs_compute_config(connector, pipe_config, > > > link_bpp_x16); > > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, > > > conn_state); > > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, > > > pipe_config, conn_state); > > > @@ -6612,6 +6614,8 @@ static bool intel_edp_init_connector(struct > > > intel_dp *intel_dp, > > > > > > intel_pps_init_late(intel_dp); > > > > > > + intel_dp->alpm_parameters.auxless_alpm_supported = > > > intel_alpm_get_aux_less_status(intel_dp); > > > > How about aux_wake ? > > I felt auxless_alpm_supported will give both aux-less and aux-wake > info, if it is true then aux-less otherwise aux-wake. Do you see any > issue here? If panel supports only AUX Wake? BR, Jouni Högander > > Regards, > Animesh > > > > > + > > > return true; > > > > > > out_vdd_off: >
diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c index 13bac3e8c8fa..3bb69ed16aab 100644 --- a/drivers/gpu/drm/i915/display/intel_alpm.c +++ b/drivers/gpu/drm/i915/display/intel_alpm.c @@ -11,6 +11,16 @@ #include "intel_dp_aux.h" #include "intel_psr_regs.h" +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp) +{ + u8 alpm_caps = 0; + + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP, + &alpm_caps) != 1) + return false; + return alpm_caps & DP_ALPM_AUX_LESS_CAP; +} + /* * See Bspec: 71632 for the table * @@ -242,6 +252,44 @@ bool intel_alpm_compute_params(struct intel_dp *intel_dp, return true; } +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_i915_private *i915 = dp_to_i915(intel_dp); + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; + int waketime_in_lines, first_sdp_position; + int context_latency, guardband; + + crtc_state->has_lobf = false; + + if (!intel_dp_is_edp(intel_dp)) + return; + + if (DISPLAY_VER(i915) < 20) + return; + + if (!intel_dp_as_sdp_supported(intel_dp)) + return; + + if (crtc_state->has_psr) + return; + + if (intel_alpm_compute_params(intel_dp, crtc_state)) { + context_latency = adjusted_mode->crtc_vblank_start - adjusted_mode->crtc_vdisplay; + guardband = adjusted_mode->crtc_vtotal - + adjusted_mode->crtc_vdisplay - context_latency; + first_sdp_position = adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vsync_start; + if (intel_dp->alpm_parameters.auxless_alpm_supported) + waketime_in_lines = intel_dp->alpm_parameters.io_wake_lines; + else + waketime_in_lines = intel_dp->alpm_parameters.fast_wake_lines; + + if ((context_latency + guardband) > (first_sdp_position + waketime_in_lines)) + crtc_state->has_lobf = true; + } +} + static void lnl_alpm_configure(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h b/drivers/gpu/drm/i915/display/intel_alpm.h index c45d078e5a6b..b9602b71d28f 100644 --- a/drivers/gpu/drm/i915/display/intel_alpm.h +++ b/drivers/gpu/drm/i915/display/intel_alpm.h @@ -10,9 +10,14 @@ struct intel_dp; struct intel_crtc_state; +struct drm_connector_state; +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); bool intel_alpm_compute_params(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state); +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state, + struct drm_connector_state *conn_state); void intel_alpm_configure(struct intel_dp *intel_dp); #endif diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index d94f50c6dc6c..5a0ffd5aa48b 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1405,6 +1405,9 @@ struct intel_crtc_state { /* for loading single buffered registers during vblank */ struct drm_vblank_work vblank_work; + + /* LOBF flag */ + bool has_lobf; }; enum intel_pipe_crc_source { @@ -1835,6 +1838,7 @@ struct intel_dp { u8 fast_wake_lines; /* LNL and beyond */ + bool auxless_alpm_supported; u8 check_entry_lines; u8 silence_period_sym_clocks; u8 lfps_half_cycle_num_of_syms; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index e05e25cd4a94..563739c6014c 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -48,6 +48,7 @@ #include "i915_drv.h" #include "i915_irq.h" #include "i915_reg.h" +#include "intel_alpm.h" #include "intel_atomic.h" #include "intel_audio.h" #include "intel_backlight.h" @@ -2997,6 +2998,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, intel_vrr_compute_config(pipe_config, conn_state); intel_dp_compute_as_sdp(intel_dp, pipe_config); intel_psr_compute_config(intel_dp, pipe_config, conn_state); + intel_alpm_compute_lobf_config(intel_dp, pipe_config, conn_state); intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16); intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); @@ -6612,6 +6614,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, intel_pps_init_late(intel_dp); + intel_dp->alpm_parameters.auxless_alpm_supported = intel_alpm_get_aux_less_status(intel_dp); + return true; out_vdd_off:
Link Off Between Active Frames, is a new feature for eDP that allows the panel to go to lower power state after transmission of data. This is a feature on top of ALPM, AS SDP. Add compute config during atomic-check phase. v1: RFC version. v2: Add separate flag for auxless-alpm. [Jani] v3: - intel_dp->lobf_supported replaced with crtc_state->has_lobf. [Jouni] - Add DISPLAY_VER() check. [Jouni] - Modify function name of get_aux_less_status. [Jani] Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_alpm.c | 48 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_alpm.h | 5 ++ .../drm/i915/display/intel_display_types.h | 4 ++ drivers/gpu/drm/i915/display/intel_dp.c | 4 ++ 4 files changed, 61 insertions(+)