Re: [PATCH 2/8] drm/i915/display: Store compressed bpp in U6.4 format
Hi Mitul,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-tip/drm-tip]
url:
https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-display-dp-Add-helper-function-to-get-DSC-bpp-precision/20230929-162949
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:
https://lore.kernel.org/r/20230929071322.945521-3-mitulkumar.ajitkumar.golani%40intel.com
patch subject: [PATCH 2/8] drm/i915/display: Store compressed bpp in U6.4 format
config: x86_64-rhel-8.3-kselftests
(https://download.01.org/0day-ci/archive/20230930/202309301303.ujzmuwzh-...@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20230930/202309301303.ujzmuwzh-...@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
| Closes:
https://lore.kernel.org/oe-kbuild-all/202309301303.ujzmuwzh-...@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_link_bw.c: In function
'intel_link_bw_reduce_bpp':
>> drivers/gpu/drm/i915/display/intel_link_bw.c:71:52: error: 'struct
>> ' has no member named 'compressed_bpp'; did you mean
>> 'compressed_bpp_x16'?
71 | link_bpp = crtc_state->dsc.compressed_bpp;
|^~
|compressed_bpp_x16
vim +71 drivers/gpu/drm/i915/display/intel_link_bw.c
8ca0b875c08258 Imre Deak 2023-09-21 28
8ca0b875c08258 Imre Deak 2023-09-21 29 /**
8ca0b875c08258 Imre Deak 2023-09-21 30 * intel_link_bw_reduce_bpp - reduce
maximum link bpp for a selected pipe
8ca0b875c08258 Imre Deak 2023-09-21 31 * @state: atomic state
8ca0b875c08258 Imre Deak 2023-09-21 32 * @limits: link BW limits
8ca0b875c08258 Imre Deak 2023-09-21 33 * @pipe_mask: mask of pipes to select
from
8ca0b875c08258 Imre Deak 2023-09-21 34 * @reason: explanation of why bpp
reduction is needed
8ca0b875c08258 Imre Deak 2023-09-21 35 *
8ca0b875c08258 Imre Deak 2023-09-21 36 * Select the pipe from @pipe_mask
with the biggest link bpp value and set the
8ca0b875c08258 Imre Deak 2023-09-21 37 * maximum of link bpp in @limits
below this value. Modeset the selected pipe,
8ca0b875c08258 Imre Deak 2023-09-21 38 * so that its state will get
recomputed.
8ca0b875c08258 Imre Deak 2023-09-21 39 *
8ca0b875c08258 Imre Deak 2023-09-21 40 * This function can be called to
resolve a link's BW overallocation by reducing
8ca0b875c08258 Imre Deak 2023-09-21 41 * the link bpp of one pipe on the
link and hence reducing the total link BW.
8ca0b875c08258 Imre Deak 2023-09-21 42 *
8ca0b875c08258 Imre Deak 2023-09-21 43 * Returns
8ca0b875c08258 Imre Deak 2023-09-21 44 * - 0 in case of success
8ca0b875c08258 Imre Deak 2023-09-21 45 * - %-ENOSPC if no pipe can further
reduce its link bpp
8ca0b875c08258 Imre Deak 2023-09-21 46 * - Other negative error, if
modesetting the selected pipe failed
8ca0b875c08258 Imre Deak 2023-09-21 47 */
8ca0b875c08258 Imre Deak 2023-09-21 48 int intel_link_bw_reduce_bpp(struct
intel_atomic_state *state,
8ca0b875c08258 Imre Deak 2023-09-21 49 struct
intel_link_bw_limits *limits,
8ca0b875c08258 Imre Deak 2023-09-21 50 u8
pipe_mask,
8ca0b875c08258 Imre Deak 2023-09-21 51 const char
*reason)
8ca0b875c08258 Imre Deak 2023-09-21 52 {
8ca0b875c08258 Imre Deak 2023-09-21 53 struct drm_i915_private *i915 =
to_i915(state->base.dev);
8ca0b875c08258 Imre Deak 2023-09-21 54 enum pipe max_bpp_pipe =
INVALID_PIPE;
8ca0b875c08258 Imre Deak 2023-09-21 55 struct intel_crtc *crtc;
8ca0b875c08258 Imre Deak 2023-09-21 56 int max_bpp = 0;
8ca0b875c08258 Imre Deak 2023-09-21 57
8ca0b875c08258 Imre Deak 2023-09-21 58
for_each_intel_crtc_in_pipe_mask(>drm, crtc, pipe_mask) {
8ca0b875c08258 Imre Deak 2023-09-21 59 struct intel_crtc_state
*crtc_state;
8ca0b875c08258 Imre Deak 2023-09-21 60 int link_bpp;
8ca0b875c08258 Imre Deak 2023-09-21 61
8ca0b875c08258 Imre Deak 2023-09-21 62 if
(limits->bpp_limit_reached_pipes & BIT(crtc->pipe))
8ca0b875c08258 Imre Deak 2023-09-21 63 continue;
8ca0b875c08258 Imre Deak 2023-09-21 64
8ca0b875c08258 Imre Deak 2023-09-21 65 crtc_state =
intel_atomic_get_crtc_state(>base,
8ca0b875c08258 Imre Deak 2023-09-21 66
crtc);
8ca0b875c08258 Imre Deak 2023-09-21 67 if (IS_ERR(crtc_state))
8ca0b875c08258 Imre Deak 2023-09-21 68 return
PTR_ERR(crtc_s
Re: [PATCH v3] backlight: pwm_bl: Disable PWM on shutdown and suspend
Hi Uwe,
kernel test robot noticed the following build errors:
[auto build test ERROR on 8fff9184d1b5810dca5dd1a02726d4f844af88fc]
url:
https://github.com/intel-lab-lkp/linux/commits/Uwe-Kleine-K-nig/backlight-pwm_bl-Disable-PWM-on-shutdown-and-suspend/20230926-230323
base: 8fff9184d1b5810dca5dd1a02726d4f844af88fc
patch link:
https://lore.kernel.org/r/20230926150116.2124384-1-u.kleine-koenig%40pengutronix.de
patch subject: [PATCH v3] backlight: pwm_bl: Disable PWM on shutdown and suspend
config: mips-allmodconfig
(https://download.01.org/0day-ci/archive/20230930/202309301206.ot7dbpq7-...@intel.com/config)
compiler: mips-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20230930/202309301206.ot7dbpq7-...@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
| Closes:
https://lore.kernel.org/oe-kbuild-all/202309301206.ot7dbpq7-...@intel.com/
All errors (new ones prefixed by >>):
drivers/video/backlight/pwm_bl.c: In function 'pwm_backlight_remove':
>> drivers/video/backlight/pwm_bl.c:632:33: error: 'state' undeclared (first
>> use in this function); did you mean 'statx'?
632 | pwm_get_state(pb->pwm, );
| ^
| statx
drivers/video/backlight/pwm_bl.c:632:33: note: each undeclared identifier is
reported only once for each function it appears in
vim +632 drivers/video/backlight/pwm_bl.c
624
625 static void pwm_backlight_remove(struct platform_device *pdev)
626 {
627 struct backlight_device *bl = platform_get_drvdata(pdev);
628 struct pwm_bl_data *pb = bl_get_data(bl);
629
630 backlight_device_unregister(bl);
631 pwm_backlight_power_off(pb);
> 632 pwm_get_state(pb->pwm, );
633 state.duty_cycle = 0;
634 state.enabled = false;
635 pwm_apply_state(pb->pwm, );
636
637 if (pb->exit)
638 pb->exit(>dev);
639 }
640
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 11/15] platform/x86/amd/pmf: dump policy binary data
On 9/23/2023 12:31 AM, Mario Limonciello wrote:
> On 9/22/2023 12:50, Shyam Sundar S K wrote:
>> Sometimes policy binary retrieved from the BIOS maybe incorrect that
>> can
>> end up in failing to enable the Smart PC solution feature.
>>
>> Use print_hex_dump_debug() to dump the policy binary in hex, so that we
>> debug the issues related to the binary even before sending that to TA.
>>
>> Signed-off-by: Shyam Sundar S K
>> ---
>> drivers/platform/x86/amd/pmf/tee-if.c | 7 +++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>> b/drivers/platform/x86/amd/pmf/tee-if.c
>> index fa37cfab2dc7..3daa122f35d5 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -290,6 +290,9 @@ static ssize_t amd_pmf_get_pb_data(struct file
>> *filp, const char __user *buf,
>> if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
>> return -EFAULT;
>> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1,
>> dev->policy_buf,
>> + dev->policy_sz, false);
>> +
>
> Should this one also be guarded by CONFIG_AMD_PMF_DEBUG or no?
No, as this dump is called from amd_pmf_get_pb_data() which is already
under CONFIG_AMD_PMF_DEBUG.
>
>> ret = amd_pmf_start_policy_engine(dev);
>> if (ret)
>> return -EINVAL;
>> @@ -329,6 +332,10 @@ static int amd_pmf_get_bios_buffer(struct
>> amd_pmf_dev *dev)
>> return -ENOMEM;
>> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>> +#ifdef CONFIG_AMD_PMF_DEBUG
>> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1,
>> dev->policy_buf,
>> + dev->policy_sz, false);
>> +#endif
>> #ifdef CONFIG_AMD_PMF_DEBUG
>> if (pb_side_load)
>
> It looks like you can combine the two #ifdef from the previous patches.
Thank you, I have addressed this in v2.
Thanks,
Shyam
Re: [PATCH 04/15] platform/x86/amd/pmf: Add support for PMF Policy Binary
On 9/23/2023 12:21 AM, Mario Limonciello wrote:
> On 9/22/2023 12:50, Shyam Sundar S K wrote:
>> PMF Policy binary is a encrypted and signed binary that will be part
>> of the BIOS. PMF driver via the ACPI interface checks the existence
>> of Smart PC bit. If the advertised bit is found, PMF driver walks
>> the acpi namespace to find out the policy binary size and the address
>> which has to be passed to the TA during the TA init sequence.
>>
>> The policy binary is comprised of inputs (or the events) and outputs
>> (or the actions). With the PMF ecosystem, OEMs generate the policy
>> binary (or could be multiple binaries) that contains a supported set
>> of inputs and outputs which could be specifically carved out for each
>> usage segment (or for each user also) that could influence the system
>> behavior either by enriching the user experience or/and boost/throttle
>> power limits.
>>
>> Once the TA init command succeeds, the PMF driver sends the changing
>> events in the current environment to the TA for a constant sampling
>> frequency time (the event here could be a lid close or open) and
>> if the policy binary has corresponding action built within it, the
>> TA sends the action for it in the subsequent enact command.
>>
>> If the inputs sent to the TA has no output defined in the policy
>> binary generated by OEMs, there will be no action to be performed
>> by the PMF driver.
>>
>> Example policies:
>>
>> 1) if slider is performance ; set the SPL to 40W
>> Here PMF driver registers with the platform profile interface and
>> when the slider position is changed, PMF driver lets the TA know
>> about this. TA sends back an action to update the Sustained
>> Power Limit (SPL). PMF driver updates this limit via the PMFW mailbox.
>>
>> 2) if user_away ; then lock the system
>> Here PMF driver hooks to the AMD SFH driver to know the user presence
>> and send the inputs to TA and if the condition is met, the TA sends
>> the action of locking the system. PMF driver generates a uevent and
>> based on the udev rule in the userland the system gets locked with
>> systemctl.
>>
>> The intent here is to provide the OEM's to make a policy to lock the
>> system when the user is away ; but the userland can make a choice to
>> ignore it.
>>
>> and so on.
>>
>> The OEMs will have an utility to create numerous such policies and
>> the policies shall be reviewed by AMD before signing and encrypting
>> them. Policies are shared between operating systems to have seemless
>> user
>> experience.
>>
>> Since all this action has to happen via the "amdtee" driver, currently
>> there is no caller for it in the kernel which can load the amdtee
>> driver.
>> Without amdtee driver loading onto the system the "tee" calls shall
>> fail
>> from the PMF driver. Hence an explicit "request_module" has been added
>> to address this.
>>
>> Signed-off-by: Shyam Sundar S K
>> ---
>> drivers/platform/x86/amd/pmf/Kconfig | 1 +
>> drivers/platform/x86/amd/pmf/acpi.c | 37 +++
>> drivers/platform/x86/amd/pmf/core.c | 12 +++
>> drivers/platform/x86/amd/pmf/pmf.h | 132
>> drivers/platform/x86/amd/pmf/tee-if.c | 141
>> +-
>> 5 files changed, 321 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig
>> b/drivers/platform/x86/amd/pmf/Kconfig
>> index 3064bc8ea167..437b78c6d1c5 100644
>> --- a/drivers/platform/x86/amd/pmf/Kconfig
>> +++ b/drivers/platform/x86/amd/pmf/Kconfig
>> @@ -9,6 +9,7 @@ config AMD_PMF
>> depends on POWER_SUPPLY
>> depends on AMD_NB
>> select ACPI_PLATFORM_PROFILE
>> + depends on AMDTEE
>> help
>> This driver provides support for the AMD Platform Management
>> Framework.
>> The goal is to enhance end user experience by making AMD PCs
>> smarter,
>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c
>> b/drivers/platform/x86/amd/pmf/acpi.c
>> index 3fc5e4547d9f..d0512af2cd42 100644
>> --- a/drivers/platform/x86/amd/pmf/acpi.c
>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>> @@ -286,6 +286,43 @@ int apmf_install_handler(struct amd_pmf_dev
>> *pmf_dev)
>> return 0;
>> }
>> +static acpi_status apmf_walk_resources(struct acpi_resource *res,
>> void *data)
>> +{
>> + struct amd_pmf_dev *dev = data;
>> +
>> + switch (res->type) {
>> + case ACPI_RESOURCE_TYPE_ADDRESS64:
>> + dev->policy_addr = res->data.address64.address.minimum;
>> + dev->policy_sz = res->data.address64.address.address_length;
>> + break;
>> + case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
>> + dev->policy_addr = res->data.fixed_memory32.address;
>> + dev->policy_sz = res->data.fixed_memory32.address_length;
>> + break;
>> + }
>> +
>> + if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ ||
>> dev->policy_sz == 0) {
>> + pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
>> + return AE_ERROR;
>> + }
>> +
>> + return AE_OK;
Re: ti-sn65dsi86 linux driver using dsi clock source for pll
Hi,
On Fri, Sep 29, 2023 at 2:50 PM Laurent Pinchart
wrote:
>
> Hi Doug,
>
> CC'ing the dri-devel mailing list and Douglas Anderson.
>
> On Fri, Sep 29, 2023 at 03:36:52PM -0400, Douglas Cooper wrote:
> > Hello,
> >
> > I've been trying to get the ti-sn65dsi86 to work with the dsi bus as the clk
> > source (refclk grounded) and have concluded that the pll is not getting
> > locked.
> > Assuming the hardware is sound, have you ever seen this topology evaluated
> > before? I'm questioning if that was a use case considered in the driver
> > development. I will continue to actively investigate this.
>
> I don't think I've tested this mode, sorry. Maybe other people on the
> list have some experience with this.
I wouldn't suggest using this mode unless you have no choice.
>From my recollection we tried to use this mode on the very first
prototype board of sdm845-cheza. It turned out to be _very_ limiting
and it couldn't properly meet the timing requirements of the panel we
were using. I think someone hacked things up temporarily by
underdriving the panel at a much lower refresh rate and we eventually
got it working, but we quickly abandoned trying to use ti-sn65dsi86 in
this mode and threw away all of those old prototype boards.
Since then, I've _tried_ to keep the code in ti-sn65dsi86 supporting
this mode alive, but I'm not super confident in it.
One thing that sticks out in particular in my mind is a bit of code in
ti_sn65dsi86_resume(). You can see there that we don't call
ti_sn65dsi86_enable_comms() if there's no reference clock. I believe
that I added this code more out of guessing than anything else, since
I don't recall this being well documented in the manual and, when the
code was added, the early prototypes of cheza were long, long gone. I
believe (?) I guessed this by seeing that I couldn't do things like
AUX channel transfers without configuring the PLL and the PLL was
based on the reference clock. Ah, here ya go. I documented my thought
process in commit b137406d9679 ("drm/bridge: ti-sn65dsi86: If refclk,
DP AUX can happen w/out pre-enable"). Though looking at it now, it
seems odd that the code waiting for the PLL to lock doesn't happen
until ti_sn_link_training(). Hmmm...
If you have tested and see that things work differently than I
guessed, feel free to send up a patch!
One thing to note is that if, indeed, you need a reference clock
before you can do AUX transactions then it's going to be really hard
to make this work together with the generic "edp-panel". Specifically
you'd need to turn on the MIPI source clock _before_ you can read the
EDID of the panel, but without the EDID you won't really know what
MIPI clock you should use. :-/
-Doug
Re: [PATCH v4 09/10] drm/sched: Add helper to queue TDR immediately for current and future jobs
On 2023-09-19 01:01, Matthew Brost wrote:
> Add helper to queue TDR immediately for current and future jobs. This
> will be used in XE, new Intel GPU driver, to trigger the TDR to cleanup
Please use present tense, "is", in code, comments, commits, etc.
Is it "XE" or is it "Xe"? I always thought it was "Xe".
This is used in Xe, a new Intel GPU driver, to trigger a TDR to clean up
Code, comments, commits, etc., immediately become history, and it's a bit
ambitious to use future tense in something which immediately becomes
history. It's much better to describe what is happening now, including the patch
in question (any patch, ftm) is considered "now"/"current state" as well.
> a drm_scheduler that encounter error[.]>
> v2:
> - Drop timeout args, rename function, use mod delayed work (Luben)
>
> Signed-off-by: Matthew Brost
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 19 ++-
> include/drm/gpu_scheduler.h| 1 +
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index e8a3e6033f66..88ef8be2d3c7 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -435,7 +435,7 @@ static void drm_sched_start_timeout(struct
> drm_gpu_scheduler *sched)
>
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> !list_empty(>pending_list))
> - queue_delayed_work(sched->timeout_wq, >work_tdr,
> sched->timeout);
> + mod_delayed_work(sched->timeout_wq, >work_tdr,
> sched->timeout);
> }
>
> static void drm_sched_start_timeout_unlocked(struct drm_gpu_scheduler *sched)
> @@ -445,6 +445,23 @@ static void drm_sched_start_timeout_unlocked(struct
> drm_gpu_scheduler *sched)
> spin_unlock(>job_list_lock);
> }
>
> +/**
> + * drm_sched_tdr_queue_imm: - immediately start timeout handler including
> future
> + * jobs
Let's not mention "including future jobs", since we don't know the future.
But we can sneak "jobs" into the description like this:
* drm_sched_tdr_queue_imm - immediately start job timeout handler
:-)
> + *
> + * @sched: scheduler where the timeout handling should be started.
"where" --> "for which"
The former denotes a location, like in space-time, and the latter
denotes an object, like a scheduler, a spaceship, a bicycle, etc.
> + *
> + * Start timeout handling immediately for current and future jobs
* Start timeout handling immediately for the named scheduler.
> + */
> +void drm_sched_tdr_queue_imm(struct drm_gpu_scheduler *sched)
> +{
> + spin_lock(>job_list_lock);
> + sched->timeout = 0;
> + drm_sched_start_timeout(sched);
> + spin_unlock(>job_list_lock);
> +}
> +EXPORT_SYMBOL(drm_sched_tdr_queue_imm);
> +
> /**
> * drm_sched_fault - immediately start timeout handler
> *
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 7e6c121003ca..27f5778bbd6d 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -568,6 +568,7 @@ void drm_sched_entity_modify_sched(struct
> drm_sched_entity *entity,
> struct drm_gpu_scheduler **sched_list,
> unsigned int num_sched_list);
>
> +void drm_sched_tdr_queue_imm(struct drm_gpu_scheduler *sched);
> void drm_sched_job_cleanup(struct drm_sched_job *job);
> void drm_sched_wakeup_if_can_queue(struct drm_gpu_scheduler *sched);
> bool drm_sched_submit_ready(struct drm_gpu_scheduler *sched);
Looks good!
Fix the above, for an immediate R-B. :-)
--
Regards,
Luben
Re: [PATCH v4 08/10] drm/sched: Submit job before starting TDR
Hi,
On 2023-09-19 01:01, Matthew Brost wrote:
> If the TDR is set to a value, it can fire before a job is submitted in
> drm_sched_main. The job should be always be submitted before the TDR
> fires, fix this ordering.
>
> v2:
> - Add to pending list before run_job, start TDR after (Luben, Boris)
>
> Signed-off-by: Matthew Brost
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index a5cc9b6c2faa..e8a3e6033f66 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -517,7 +517,6 @@ static void drm_sched_job_begin(struct drm_sched_job
> *s_job)
>
> spin_lock(>job_list_lock);
> list_add_tail(_job->list, >pending_list);
> - drm_sched_start_timeout(sched);
> spin_unlock(>job_list_lock);
> }
>
> @@ -1138,6 +1137,7 @@ static void drm_sched_run_job_work(struct work_struct
> *w)
> fence = sched->ops->run_job(sched_job);
> complete_all(>entity_idle);
> drm_sched_fence_scheduled(s_fence, fence);
> + drm_sched_start_timeout_unlocked(sched);
>
> if (!IS_ERR_OR_NULL(fence)) {
> /* Drop for original kref_init of the fence */
No.
See Message-ID: ,
and Message-ID: <8e5eab14-9e55-42c9-b6ea-02fcc5912...@amd.com>,
and Message-ID: <24bc965f-61fb-4b92-9afa-360ca85a5...@amd.com>.
--
Regards,
Luben
Re: [PATCH v4 07/10] drm/sched: Start submission before TDR in drm_sched_start
Hi, On 2023-09-19 01:01, Matthew Brost wrote: > If the TDR is set to a very small value it can fire before the > submission is started in the function drm_sched_start. The submission is > expected to running when the TDR fires, fix this ordering so this > expectation is always met. > > Signed-off-by: Matthew Brost > --- > drivers/gpu/drm/scheduler/sched_main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 09ef07b9e9d5..a5cc9b6c2faa 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -684,10 +684,10 @@ void drm_sched_start(struct drm_gpu_scheduler *sched, > bool full_recovery) > drm_sched_job_done(s_job, -ECANCELED); > } > > + drm_sched_submit_start(sched); > + > if (full_recovery) > drm_sched_start_timeout_unlocked(sched); > - > - drm_sched_submit_start(sched); > } > EXPORT_SYMBOL(drm_sched_start); No. A timeout timer should be started before we submit anything down to the hardware. See Message-ID: , and Message-ID: <8e5eab14-9e55-42c9-b6ea-02fcc5912...@amd.com>. You shouldn't start TDR at an arbitrarily late time after job submission to the hardware. To close this, the timer is started before jobs are submitted to the hardware. One possibility is to increase the timeout timer value. -- Regards, Luben
Re: ti-sn65dsi86 linux driver using dsi clock source for pll
Hi Doug, CC'ing the dri-devel mailing list and Douglas Anderson. On Fri, Sep 29, 2023 at 03:36:52PM -0400, Douglas Cooper wrote: > Hello, > > I've been trying to get the ti-sn65dsi86 to work with the dsi bus as the clk > source (refclk grounded) and have concluded that the pll is not getting > locked. > Assuming the hardware is sound, have you ever seen this topology evaluated > before? I'm questioning if that was a use case considered in the driver > development. I will continue to actively investigate this. I don't think I've tested this mode, sorry. Maybe other people on the list have some experience with this. -- Regards, Laurent Pinchart
Re: [PATCH] drm/bridge: adv7511: Convert to use maple tree register cache
Hi Mark,
Thank you for the patch.
On Fri, Sep 29, 2023 at 02:54:19PM +0200, Mark Brown wrote:
> The maple tree register cache is based on a much more modern data structure
> than the rbtree cache and makes optimisation choices which are probably
> more appropriate for modern systems than those made by the rbtree cache.
I trust on your this statement.
> Signed-off-by: Mark Brown
Reviewed-by: Laurent Pinchart
Out of curiosity, is this part of an effort to drop the rbtree cache ?
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 2611afd2c1c1..d518de88b5c3 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -121,7 +121,7 @@ static const struct regmap_config adv7511_regmap_config =
> {
> .val_bits = 8,
>
> .max_register = 0xff,
> - .cache_type = REGCACHE_RBTREE,
> + .cache_type = REGCACHE_MAPLE,
> .reg_defaults_raw = adv7511_register_defaults,
> .num_reg_defaults_raw = ARRAY_SIZE(adv7511_register_defaults),
>
> @@ -1068,7 +1068,7 @@ static const struct regmap_config
> adv7511_cec_regmap_config = {
> .val_bits = 8,
>
> .max_register = 0xff,
> - .cache_type = REGCACHE_RBTREE,
> + .cache_type = REGCACHE_MAPLE,
> .volatile_reg = adv7511_cec_register_volatile,
> };
>
>
> ---
> base-commit: 6465e260f48790807eef06b583b38ca9789b6072
> change-id: 20230929-drm-adv7511-2d592921f8a2
--
Regards,
Laurent Pinchart
Re: [RFC PATCH 1/3] drm/msm/dpu: add support for MSM8953
On Fri, 29 Sept 2023 at 23:53, Luca Weiss wrote: > > On Samstag, 23. September 2023 23:49:10 CEST Dmitry Baryshkov wrote: > > Experimental support for MSM8953, which has MDP5 v1.16. It looks like > > trimmed down version of MSM8996. Less SSPP, LM and PP blocks. No DSC, > > etc. > > > > Hi Dmitry, > > As written on IRC, on sdm632-fairphone-fp3 with this DPU patches the screen is > initializing and displaying stuff :) But there's some errors, which presumably > are the reason that the screen is only updating a few times per second. > > [ 22.774205] [drm:dpu_kms_hw_init:1164] dpu hardware revision:0x1010 > [ 23.099806] [drm:_dpu_encoder_phys_cmd_wait_for_ctl_start:657] [dpu > error]enc31 intf1 ctl start interrupt wait failed > [ 23.099821] [drm:dpu_kms_wait_for_commit_done:495] [dpu error]wait for > commit done returned -22 > > These messages appear about 13 times per second but as I mentioned, the screen > *is* updating (slowly) there. Thank you for the testing, I'll see if I can determine what is causing the ctl start issue. > > Also you for sure forgot to add "qcom,msm8953-mdp5" to the > msm_mdp5_dpu_migration list, without this DPU is never even considered for > 8953. Yep. > > Regards > Luca -- With best wishes Dmitry
Re: [PATCH v4 06/10] drm/sched: Add drm_sched_start_timeout_unlocked helper
Hi,
On 2023-09-19 01:01, Matthew Brost wrote:
> Also add a lockdep assert to drm_sched_start_timeout.
>
> Signed-off-by: Matthew Brost
Reviewed-by: Luben Tuikov
Thanks for this patch!
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 23 +--
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index 1e21d234fb5c..09ef07b9e9d5 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -431,11 +431,20 @@ static void drm_sched_job_done_cb(struct dma_fence *f,
> struct dma_fence_cb *cb)
> */
> static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched)
> {
> + lockdep_assert_held(>job_list_lock);
> +
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> !list_empty(>pending_list))
> queue_delayed_work(sched->timeout_wq, >work_tdr,
> sched->timeout);
> }
>
> +static void drm_sched_start_timeout_unlocked(struct drm_gpu_scheduler *sched)
> +{
> + spin_lock(>job_list_lock);
> + drm_sched_start_timeout(sched);
> + spin_unlock(>job_list_lock);
> +}
> +
> /**
> * drm_sched_fault - immediately start timeout handler
> *
> @@ -548,11 +557,8 @@ static void drm_sched_job_timedout(struct work_struct
> *work)
> spin_unlock(>job_list_lock);
> }
>
> - if (status != DRM_GPU_SCHED_STAT_ENODEV) {
> - spin_lock(>job_list_lock);
> - drm_sched_start_timeout(sched);
> - spin_unlock(>job_list_lock);
> - }
> + if (status != DRM_GPU_SCHED_STAT_ENODEV)
> + drm_sched_start_timeout_unlocked(sched);
> }
>
> /**
> @@ -678,11 +684,8 @@ void drm_sched_start(struct drm_gpu_scheduler *sched,
> bool full_recovery)
> drm_sched_job_done(s_job, -ECANCELED);
> }
>
> - if (full_recovery) {
> - spin_lock(>job_list_lock);
> - drm_sched_start_timeout(sched);
> - spin_unlock(>job_list_lock);
> - }
> + if (full_recovery)
> + drm_sched_start_timeout_unlocked(sched);
>
> drm_sched_submit_start(sched);
> }
--
Regards,
Luben
Re: [RFC PATCH 1/3] drm/msm/dpu: add support for MSM8953
On Samstag, 23. September 2023 23:49:10 CEST Dmitry Baryshkov wrote:
> Experimental support for MSM8953, which has MDP5 v1.16. It looks like
> trimmed down version of MSM8996. Less SSPP, LM and PP blocks. No DSC,
> etc.
>
Hi Dmitry,
As written on IRC, on sdm632-fairphone-fp3 with this DPU patches the screen is
initializing and displaying stuff :) But there's some errors, which presumably
are the reason that the screen is only updating a few times per second.
[ 22.774205] [drm:dpu_kms_hw_init:1164] dpu hardware revision:0x1010
[ 23.099806] [drm:_dpu_encoder_phys_cmd_wait_for_ctl_start:657] [dpu
error]enc31 intf1 ctl start interrupt wait failed
[ 23.099821] [drm:dpu_kms_wait_for_commit_done:495] [dpu error]wait for
commit done returned -22
These messages appear about 13 times per second but as I mentioned, the screen
*is* updating (slowly) there.
Also you for sure forgot to add "qcom,msm8953-mdp5" to the
msm_mdp5_dpu_migration list, without this DPU is never even considered for
8953.
Regards
Luca
> Signed-off-by: Dmitry Baryshkov
> ---
> .../msm/disp/dpu1/catalog/dpu_1_16_msm8953.h | 221 ++
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 12 +
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
> 4 files changed, 235 insertions(+)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_16_msm8953.h
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_16_msm8953.h
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_16_msm8953.h new file mode
> 100644
> index ..6944bfa4568a
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_16_msm8953.h
> @@ -0,0 +1,221 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#ifndef _DPU_1_16_MSM8953_H
> +#define _DPU_1_16_MSM8953_H
> +
> +static const struct dpu_caps msm8953_dpu_caps = {
> + .max_mixer_width = DEFAULT_DPU_LINE_WIDTH,
> + .max_mixer_blendstages = 0x4,
> + .max_linewidth = DEFAULT_DPU_LINE_WIDTH,
> + .pixel_ram_size = 40 * 1024,
> + .max_hdeci_exp = MAX_HORZ_DECIMATION,
> + .max_vdeci_exp = MAX_VERT_DECIMATION,
> +};
> +
> +static const struct dpu_mdp_cfg msm8953_mdp[] = {
> + {
> + .name = "top_0",
> + .base = 0x0, .len = 0x454,
> + .features = BIT(DPU_MDP_VSYNC_SEL),
> + .clk_ctrls = {
> + [DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0
> },
> + [DPU_CLK_CTRL_RGB0] = { .reg_off = 0x2ac, .bit_off = 4
> },
> + [DPU_CLK_CTRL_RGB1] = { .reg_off = 0x2b4, .bit_off = 4
> },
> + [DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8
> },
> + [DPU_CLK_CTRL_CURSOR0] = { .reg_off = 0x3a8, .bit_off =
> 16 },
> + },
> + },
> +};
> +
> +static const struct dpu_ctl_cfg msm8953_ctl[] = {
> + {
> + .name = "ctl_0", .id = CTL_0,
> + .base = 0x1000, .len = 0x64,
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
> + }, {
> + .name = "ctl_1", .id = CTL_1,
> + .base = 0x1200, .len = 0x64,
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10),
> + }, {
> + .name = "ctl_2", .id = CTL_2,
> + .base = 0x1400, .len = 0x64,
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11),
> + },
> +};
> +
> +static const struct dpu_sspp_cfg msm8953_sspp[] = {
> + {
> + .name = "sspp_0", .id = SSPP_VIG0,
> + .base = 0x4000, .len = 0x150,
> + .features = VIG_MSM8953_MASK,
> + .sblk = _vig_sblk_qseed2,
> + .xin_id = 0,
> + .type = SSPP_TYPE_VIG,
> + .clk_ctrl = DPU_CLK_CTRL_VIG0,
> + }, {
> + .name = "sspp_4", .id = SSPP_RGB0,
> + .base = 0x14000, .len = 0x150,
> + .features = RGB_MSM8953_MASK,
> + .sblk = _rgb_sblk,
> + .xin_id = 1,
> + .type = SSPP_TYPE_RGB,
> + .clk_ctrl = DPU_CLK_CTRL_RGB0,
> + }, {
> + .name = "sspp_5", .id = SSPP_RGB1,
> + .base = 0x16000, .len = 0x150,
> + .features = RGB_MSM8953_MASK,
> + .sblk = _rgb_sblk,
> + .xin_id = 5,
> + .type = SSPP_TYPE_RGB,
> + .clk_ctrl = DPU_CLK_CTRL_RGB1,
> + }, {
> + .name = "sspp_8", .id = SSPP_DMA0,
> + .base = 0x24000, .len = 0x150,
> + .features = DMA_MSM8953_MASK | BIT(DPU_SSPP_CURSOR),
> + .sblk = _dma_sblk,
> + .xin_id = 2,
> + .type = SSPP_TYPE_DMA,
> + .clk_ctrl = DPU_CLK_CTRL_DMA0,
> + },
> +};
> +
> +static const struct dpu_lm_cfg msm8953_lm[] = {
> + {
> + .name = "lm_0", .id = LM_0,
> + .base = 0x44000, .len = 0x320,
> +
[PATCH] Revert "drm/amd/display: Check all enabled planes in dm_check_crtc_cursor"
From: Ivan Lipski
This reverts commit 45e1ade04b4d60fe5df859076005779f27c4c9be.
Since, it causes the following IGT tests to fail:
kms_cursor_legacy@cursor-vs-flip.*
kms_cursor_legacy@flip-vs-cursor.*
Signed-off-by: Ivan Lipski
Signed-off-by: Hamza Mahfooz
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 32156609fbcf..49ffb4d6e9cc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10290,24 +10290,14 @@ static int dm_check_crtc_cursor(struct
drm_atomic_state *state,
* blending properties match the underlying planes'.
*/
- new_cursor_state = drm_atomic_get_plane_state(state, cursor);
- if (IS_ERR(new_cursor_state))
- return PTR_ERR(new_cursor_state);
-
- if (!new_cursor_state->fb)
+ new_cursor_state = drm_atomic_get_new_plane_state(state, cursor);
+ if (!new_cursor_state || !new_cursor_state->fb)
return 0;
dm_get_oriented_plane_size(new_cursor_state, _src_w,
_src_h);
cursor_scale_w = new_cursor_state->crtc_w * 1000 / cursor_src_w;
cursor_scale_h = new_cursor_state->crtc_h * 1000 / cursor_src_h;
- /* Need to check all enabled planes, even if this commit doesn't change
-* their state
-*/
- i = drm_atomic_add_affected_planes(state, crtc);
- if (i)
- return i;
-
for_each_new_plane_in_state_reverse(state, underlying,
new_underlying_state, i) {
/* Narrow down to non-cursor planes on the same CRTC as the
cursor */
if (new_underlying_state->crtc != crtc || underlying ==
crtc->cursor)
--
2.42.0
Re: [PATCH] drm/nouveau: fence: fix type cast warning in nouveau_fence_emit()
Reviewed-by: Lyude Paul
On Sat, 2023-09-16 at 03:14 +0200, Danilo Krummrich wrote:
> Fix the following warning.
>
> drivers/gpu/drm/nouveau/nouveau_fence.c:210:45: sparse: sparse:
> incorrect type in initializer (different address spaces)
> @@ expected struct nouveau_channel *chan
> @@ got struct nouveau_channel [noderef] __rcu *channel
>
> We're just about to emit the fence, there is nothing to protect against
> yet, hence it is safe to just cast __rcu away.
>
> Reported-by: kernel test robot
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202309140340.bwkxzadx-...@intel.com/
> Fixes: 978474dc8278 ("drm/nouveau: fence: fix undefined fence state after
> emit")
> Signed-off-by: Danilo Krummrich
> ---
> drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index 61d9e70da9fd..ca762ea55413 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -207,7 +207,7 @@ nouveau_fence_context_new(struct nouveau_channel *chan,
> struct nouveau_fence_cha
> int
> nouveau_fence_emit(struct nouveau_fence *fence)
> {
> - struct nouveau_channel *chan = fence->channel;
> + struct nouveau_channel *chan = unrcu_pointer(fence->channel);
> struct nouveau_fence_chan *fctx = chan->fence;
> struct nouveau_fence_priv *priv = (void*)chan->drm->fence;
> int ret;
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH] add kernel docs for dc_dmub_caps
Hi,
The $Subject could be improved, e.g.:
[PATCH] drm/amd/display: add kernel docs for dc_dmub_caps
On 9/29/23 03:00, Sagar Vashnav wrote:
> Add kernel documentation for the dc_dmub_caps structure.
>
> Signed-off-by: Sagar Vashnav
Reviewed-by: Randy Dunlap
Thanks.
> ---
> drivers/gpu/drm/amd/display/dc/dc.h | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h
> b/drivers/gpu/drm/amd/display/dc/dc.h
> index 8125839..14b4c50 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -208,6 +208,16 @@ struct dc_color_caps {
> struct mpc_color_caps mpc;
> };
>
> +/**
> + * struct dc_dmub_caps - DMUB (Display Microcontroller Unit) capabilities
> + * @psr: support for PSR (Power Saving State Residency)
> + * @mclk_sw: support for MCLK_SW (Memory Clock Switch)
> + * @subvp_psr: support for SUBVP PSR (Sub-Viewport Power Saving State
> Residency)
> + * @gecc_enable: GECC (Global Error Correcting Code) enablement.
> + *
> + * This structure describes the capabilities of the Display Microcontroller
> Unit (DMUB).
> + * It specifies whether certain features like PSR and MCLK_SW are supported.
> + */
> struct dc_dmub_caps {
> bool psr;
> bool mclk_sw;
--
~Randy
Re: [PATCH 0/9] drm: Annotate structs with __counted_by
On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > This is a batch of patches touching drm for preparing for the coming > implementation by GCC and Clang of the __counted_by attribute. Flexible > array members annotated with __counted_by can have their accesses > bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array > indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). > > As found with Coccinelle[1], add __counted_by to structs that would > benefit from the annotation. > > [...] Since this got Acks, I figure I should carry it in my tree. Let me know if this should go via drm instead. Applied to for-next/hardening, thanks! [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by https://git.kernel.org/kees/c/a6046ac659d6 [2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by https://git.kernel.org/kees/c/4df33089b46f [3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by https://git.kernel.org/kees/c/ffd3f823bdf6 [4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by https://git.kernel.org/kees/c/2de35a989b76 [5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by https://git.kernel.org/kees/c/188aeb08bfaa [6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by https://git.kernel.org/kees/c/59a54dc896c3 [7/9] drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by https://git.kernel.org/kees/c/5cd476de33af [8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by https://git.kernel.org/kees/c/b426f2e5356a [9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by https://git.kernel.org/kees/c/dc662fa1b0e4 Take care, -- Kees Cook
Re: [PATCH][next] drm/gud: Use size_add() in call to struct_size()
On Fri, 15 Sep 2023 12:43:20 -0600, Gustavo A. R. Silva wrote: > If, for any reason, the open-coded arithmetic causes a wraparound, the > protection that `struct_size()` adds against potential integer overflows > is defeated. Fix this by hardening call to `struct_size()` with `size_add()`. > > Applied to for-next/hardening, thanks! [1/1] drm/gud: Use size_add() in call to struct_size() https://git.kernel.org/kees/c/836ccb46073e Take care, -- Kees Cook
Re: [git pull] drm fixes for 6.6-rc4
The pull request you sent on Fri, 29 Sep 2023 11:46:12 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2023-09-29 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/6edc84bc3f8aceae74eb63684d53c17553382ec0 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH 0/2][next] nouveau/svm: Replace one-element array with flexible-array member
On Wed, 16 Aug 2023 12:03:06 -0600, Gustavo A. R. Silva wrote: > This small series aims to replace a one-element array with a > flexible-array member in struct nouveau_svm. And, while at it, > fix a checkpatch.pl error. > > Gustavo A. R. Silva (2): > nouveau/svm: Replace one-element array with flexible-array member in > struct nouveau_svm > nouveau/svm: Split assignment from if conditional > > [...] These look trivially correct and haven't had further feedback for over a month. Applied to for-next/hardening, thanks! [1/2] nouveau/svm: Replace one-element array with flexible-array member in struct nouveau_svm https://git.kernel.org/kees/c/6ad33b53c9b8 [2/2] nouveau/svm: Split assignment from if conditional https://git.kernel.org/kees/c/4cb2e89fea5f Take care, -- Kees Cook
[PATCH v8 3/5] drm/panfrost: Add fdinfo support for memory stats
A new DRM GEM object function is added so that drm_show_memory_stats can
provide more accurate memory usage numbers.
Ideally, in panfrost_gem_status, the BO's purgeable flag would be checked
after locking the driver's shrinker mutex, but drm_show_memory_stats takes
over the drm file's object handle database spinlock, so there's potential
for a race condition here.
Signed-off-by: Adrián Larumbe
Reviewed-by: Boris Brezillon
Reviewed-by: Steven Price
Reviewed-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++
drivers/gpu/drm/panfrost/panfrost_gem.c | 15 +++
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 97e5bc4a82c8..b834777b409b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -568,6 +568,8 @@ static void panfrost_show_fdinfo(struct drm_printer *p,
struct drm_file *file)
struct panfrost_device *pfdev = dev->dev_private;
panfrost_gpu_show_fdinfo(pfdev, file->driver_priv, p);
+
+ drm_show_memory_stats(p, file);
}
static const struct file_operations panfrost_drm_driver_fops = {
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 3c812fbd126f..de238b71b321 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -195,6 +195,20 @@ static int panfrost_gem_pin(struct drm_gem_object *obj)
return drm_gem_shmem_pin(>base);
}
+static enum drm_gem_object_status panfrost_gem_status(struct drm_gem_object
*obj)
+{
+ struct panfrost_gem_object *bo = to_panfrost_bo(obj);
+ enum drm_gem_object_status res = 0;
+
+ if (bo->base.pages)
+ res |= DRM_GEM_OBJECT_RESIDENT;
+
+ if (bo->base.madv == PANFROST_MADV_DONTNEED)
+ res |= DRM_GEM_OBJECT_PURGEABLE;
+
+ return res;
+}
+
static const struct drm_gem_object_funcs panfrost_gem_funcs = {
.free = panfrost_gem_free_object,
.open = panfrost_gem_open,
@@ -206,6 +220,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs
= {
.vmap = drm_gem_shmem_object_vmap,
.vunmap = drm_gem_shmem_object_vunmap,
.mmap = drm_gem_shmem_object_mmap,
+ .status = panfrost_gem_status,
.vm_ops = _gem_shmem_vm_ops,
};
--
2.42.0
[PATCH v8 5/5] drm/panfrost: Implement generic DRM object RSS reporting function
BO's RSS is updated every time new pages are allocated on demand and mapped
for the object at GPU page fault's IRQ handler, but only for heap buffers.
The reason this is unnecessary for non-heap buffers is that they are mapped
onto the GPU's VA space and backed by physical memory in their entirety at
BO creation time.
This calculation is unnecessary for imported PRIME objects, since heap
buffers cannot be exported by our driver, and the actual BO RSS size is the
one reported in its attached dmabuf structure.
Signed-off-by: Adrián Larumbe
Reviewed-by: Boris Brezillon
Reviewed-by: Steven Price
Reviewed-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/panfrost/panfrost_gem.c | 15 +++
drivers/gpu/drm/panfrost/panfrost_gem.h | 5 +
drivers/gpu/drm/panfrost/panfrost_mmu.c | 1 +
3 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index de238b71b321..0cf64456e29a 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -209,6 +209,20 @@ static enum drm_gem_object_status
panfrost_gem_status(struct drm_gem_object *obj
return res;
}
+static size_t panfrost_gem_rss(struct drm_gem_object *obj)
+{
+ struct panfrost_gem_object *bo = to_panfrost_bo(obj);
+
+ if (bo->is_heap) {
+ return bo->heap_rss_size;
+ } else if (bo->base.pages) {
+ WARN_ON(bo->heap_rss_size);
+ return bo->base.base.size;
+ }
+
+ return 0;
+}
+
static const struct drm_gem_object_funcs panfrost_gem_funcs = {
.free = panfrost_gem_free_object,
.open = panfrost_gem_open,
@@ -221,6 +235,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs
= {
.vunmap = drm_gem_shmem_object_vunmap,
.mmap = drm_gem_shmem_object_mmap,
.status = panfrost_gem_status,
+ .rss = panfrost_gem_rss,
.vm_ops = _gem_shmem_vm_ops,
};
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h
b/drivers/gpu/drm/panfrost/panfrost_gem.h
index ad2877eeeccd..13c0a8149c3a 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
@@ -36,6 +36,11 @@ struct panfrost_gem_object {
*/
atomic_t gpu_usecount;
+ /*
+* Object chunk size currently mapped onto physical memory
+*/
+ size_t heap_rss_size;
+
bool noexec :1;
bool is_heap:1;
};
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index d54d4e7b2195..846dd697c410 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -522,6 +522,7 @@ static int panfrost_mmu_map_fault_addr(struct
panfrost_device *pfdev, int as,
IOMMU_WRITE | IOMMU_READ | IOMMU_NOEXEC, sgt);
bomapping->active = true;
+ bo->heap_rss_size += SZ_2M;
dev_dbg(pfdev->dev, "mapped page fault @ AS%d %llx", as, addr);
--
2.42.0
[PATCH v8 4/5] drm/drm_file: Add DRM obj's RSS reporting function for fdinfo
Some BO's might be mapped onto physical memory chunkwise and on demand,
like Panfrost's tiler heap. In this case, even though the
drm_gem_shmem_object page array might already be allocated, only a very
small fraction of the BO is currently backed by system memory, but
drm_show_memory_stats will then proceed to add its entire virtual size to
the file's total resident size regardless.
This led to very unrealistic RSS sizes being reckoned for Panfrost, where
said tiler heap buffer is initially allocated with a virtual size of 128
MiB, but only a small part of it will eventually be backed by system memory
after successive GPU page faults.
Provide a new DRM object generic function that would allow drivers to
return a more accurate RSS and purgeable sizes for their BOs.
Signed-off-by: Adrián Larumbe
Reviewed-by: Boris Brezillon
Reviewed-by: Steven Price
Reviewed-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/drm_file.c | 8 +---
include/drm/drm_gem.h | 9 +
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 883d83bc0e3d..9a1bd8d0d785 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -930,6 +930,8 @@ void drm_show_memory_stats(struct drm_printer *p, struct
drm_file *file)
spin_lock(>table_lock);
idr_for_each_entry (>object_idr, obj, id) {
enum drm_gem_object_status s = 0;
+ size_t add_size = (obj->funcs && obj->funcs->rss) ?
+ obj->funcs->rss(obj) : obj->size;
if (obj->funcs && obj->funcs->status) {
s = obj->funcs->status(obj);
@@ -944,7 +946,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct
drm_file *file)
}
if (s & DRM_GEM_OBJECT_RESIDENT) {
- status.resident += obj->size;
+ status.resident += add_size;
} else {
/* If already purged or not yet backed by pages, don't
* count it as purgeable:
@@ -953,14 +955,14 @@ void drm_show_memory_stats(struct drm_printer *p, struct
drm_file *file)
}
if (!dma_resv_test_signaled(obj->resv,
dma_resv_usage_rw(true))) {
- status.active += obj->size;
+ status.active += add_size;
/* If still active, don't count as purgeable: */
s &= ~DRM_GEM_OBJECT_PURGEABLE;
}
if (s & DRM_GEM_OBJECT_PURGEABLE)
- status.purgeable += obj->size;
+ status.purgeable += add_size;
}
spin_unlock(>table_lock);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index bc9f6aa2f3fe..16364487fde9 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -208,6 +208,15 @@ struct drm_gem_object_funcs {
*/
enum drm_gem_object_status (*status)(struct drm_gem_object *obj);
+ /**
+* @rss:
+*
+* Return resident size of the object in physical memory.
+*
+* Called by drm_show_memory_stats().
+*/
+ size_t (*rss)(struct drm_gem_object *obj);
+
/**
* @vm_ops:
*
--
2.42.0
[PATCH v8 1/5] drm/panfrost: Add cycle count GPU register definitions
These GPU registers will be used when programming the cycle counter, which we need for providing accurate fdinfo drm-cycles values to user space. Signed-off-by: Adrián Larumbe Reviewed-by: Boris Brezillon Reviewed-by: Steven Price Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/panfrost/panfrost_regs.h | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h index 919f44ac853d..55ec807550b3 100644 --- a/drivers/gpu/drm/panfrost/panfrost_regs.h +++ b/drivers/gpu/drm/panfrost/panfrost_regs.h @@ -46,6 +46,8 @@ #define GPU_CMD_SOFT_RESET 0x01 #define GPU_CMD_PERFCNT_CLEAR0x03 #define GPU_CMD_PERFCNT_SAMPLE 0x04 +#define GPU_CMD_CYCLE_COUNT_START0x05 +#define GPU_CMD_CYCLE_COUNT_STOP 0x06 #define GPU_CMD_CLEAN_CACHES 0x07 #define GPU_CMD_CLEAN_INV_CACHES 0x08 #define GPU_STATUS 0x34 @@ -73,6 +75,9 @@ #define GPU_PRFCNT_TILER_EN0x74 #define GPU_PRFCNT_MMU_L2_EN 0x7c +#define GPU_CYCLE_COUNT_LO 0x90 +#define GPU_CYCLE_COUNT_HI 0x94 + #define GPU_THREAD_MAX_THREADS 0x0A0 /* (RO) Maximum number of threads per core */ #define GPU_THREAD_MAX_WORKGROUP_SIZE 0x0A4 /* (RO) Maximum workgroup size */ #define GPU_THREAD_MAX_BARRIER_SIZE0x0A8 /* (RO) Maximum threads waiting at a barrier */ -- 2.42.0
[PATCH v8 2/5] drm/panfrost: Add fdinfo support GPU load metrics
The drm-stats fdinfo tags made available to user space are drm-engine, drm-cycles, drm-max-freq and drm-curfreq, one per job slot. This deviates from standard practice in other DRM drivers, where a single set of key:value pairs is provided for the whole render engine. However, Panfrost has separate queues for fragment and vertex/tiler jobs, so a decision was made to calculate bus cycles and workload times separately. Maximum operating frequency is calculated at devfreq initialisation time. Current frequency is made available to user space because nvtop uses it when performing engine usage calculations. It is important to bear in mind that both GPU cycle and kernel time numbers provided are at best rough estimations, and always reported in excess from the actual figure because of two reasons: - Excess time because of the delay between the end of a job processing, the subsequent job IRQ and the actual time of the sample. - Time spent in the engine queue waiting for the GPU to pick up the next job. To avoid race conditions during enablement/disabling, a reference counting mechanism was introduced, and a job flag that tells us whether a given job increased the refcount. This is necessary, because user space can toggle cycle counting through a debugfs file, and a given job might have been in flight by the time cycle counting was disabled. The main goal of the debugfs cycle counter knob is letting tools like nvtop or IGT's gputop switch it at any time, to avoid power waste in case no engine usage measuring is necessary. Also add a documentation file explaining the possible values for fdinfo's engine keystrings and Panfrost-specific drm-curfreq- pairs. Signed-off-by: Adrián Larumbe Reviewed-by: Boris Brezillon Reviewed-by: Steven Price Reviewed-by: AngeloGioacchino Del Regno --- Documentation/gpu/drm-usage-stats.rst | 1 + Documentation/gpu/panfrost.rst | 38 ++ drivers/gpu/drm/panfrost/Makefile | 2 + drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21 drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 + drivers/gpu/drm/panfrost/panfrost_devfreq.c | 8 +++ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 3 ++ drivers/gpu/drm/panfrost/panfrost_device.c | 2 + drivers/gpu/drm/panfrost/panfrost_device.h | 13 + drivers/gpu/drm/panfrost/panfrost_drv.c | 58 - drivers/gpu/drm/panfrost/panfrost_gpu.c | 41 +++ drivers/gpu/drm/panfrost/panfrost_gpu.h | 4 ++ drivers/gpu/drm/panfrost/panfrost_job.c | 24 + drivers/gpu/drm/panfrost/panfrost_job.h | 5 ++ 14 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 Documentation/gpu/panfrost.rst create mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c create mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index fe35a291ff3e..8d963cd7c1b7 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -169,3 +169,4 @@ Driver specific implementations --- :ref:`i915-usage-stats` +:ref:`panfrost-usage-stats` diff --git a/Documentation/gpu/panfrost.rst b/Documentation/gpu/panfrost.rst new file mode 100644 index ..ecc48ba5ac11 --- /dev/null +++ b/Documentation/gpu/panfrost.rst @@ -0,0 +1,38 @@ +=== + drm/Panfrost Mali Driver +=== + +.. _panfrost-usage-stats: + +Panfrost DRM client usage stats implementation +== + +The drm/Panfrost driver implements the DRM client usage stats specification as +documented in :ref:`drm-client-usage-stats`. + +Example of the output showing the implemented key value pairs and entirety of +the currently possible format options: + +:: + pos:0 + flags: 0242 + mnt_id: 27 + ino:531 + drm-driver: panfrost + drm-client-id: 14 + drm-engine-fragment:1846584880 ns + drm-cycles-fragment:1424359409 + drm-maxfreq-fragment: 79987 Hz + drm-curfreq-fragment: 79987 Hz + drm-engine-vertex-tiler:71932239 ns + drm-cycles-vertex-tiler:52617357 + drm-maxfreq-vertex-tiler: 79987 Hz + drm-curfreq-vertex-tiler: 79987 Hz + drm-total-memory: 290 MiB + drm-shared-memory: 0 MiB + drm-active-memory: 226 MiB + drm-resident-memory:36496 KiB + drm-purgeable-memory: 128 KiB + +Possible `drm-engine-` key names are: `fragment`, and `vertex-tiler`. +`drm-curfreq-` values convey the current operating frequency for that engine. diff --git a/drivers/gpu/drm/panfrost/Makefile b/drivers/gpu/drm/panfrost/Makefile index 7da2b3f02ed9..2c01c1e7523e 100644 --- a/drivers/gpu/drm/panfrost/Makefile +++ b/drivers/gpu/drm/panfrost/Makefile @@ -12,4 +12,6 @@ panfrost-y := \
[PATCH v8 0/5] Add fdinfo support to Panfrost
This patch series adds fdinfo support to the Panfrost DRM driver. It will
display a series of key:value pairs under /proc/pid/fdinfo/fd for render
processes that open the Panfrost DRM file.
The pairs contain basic drm gpu engine and memory region information that
can either be cat by a privileged user or accessed with IGT's gputop
utility.
Changelog:
v1: https://lore.kernel.org/lkml/bb52b872-e41b-3894-285e-b52cfc849...@arm.com/T/
v2: https://lore.kernel.org/lkml/20230901084457.5bc1a...@collabora.com/T/
- Changed the way gpu cycles and engine time are calculated, using GPU
registers and taking into account potential resets.
- Split render engine values into fragment and vertex/tiler ones.
- Added more fine-grained calculation of RSS size for BO's.
- Implemente selection of drm-memory region size units.
- Removed locking of shrinker's mutex in GEM obj status function.
v3:
https://lore.kernel.org/lkml/20230905184533.959171-1-adrian.laru...@collabora.com/
- Changed fdinfo engine names to something more descriptive.;
- Mentioned GPU cycle counts aren't an exact measure.
- Handled the case when job->priv might be NULL.
- Handled 32 bit overflow of cycle register.
- Kept fdinfo drm memory stats size unit display within 10k times the
previous multiplier for more accurate BO size numbers.
- Removed special handling of Prime imported BO RSS.
- Use rss_size only for heap objects.
- Use bo->base.madv instead of specific purgeable flag.
- Fixed kernel test robot warnings.
v4:
https://lore.kernel.org/lkml/20230912084044.955864-1-adrian.laru...@collabora.com/
- Move cycle counter get and put to panfrost_job_hw_submit and
panfrost_job_handle_{err,done} for more accuracy.
- Make sure cycle counter refs are released in reset path
- Drop the model param for toggling cycle counting and do
leave it down to the debugfs file.
- Don't disable cycle counter when togglint debugfs file,
let refcounting logic handle it instead.
- Remove fdinfo data nested structure definion and 'names' field
- When incrementing BO RSS size in GPU MMU page fault IRQ handler, assume
granuality of 2MiB for every successful mapping.
- drm-file picks an fdinfo memory object size unit that doesn't lose precision.
v5:
https://lore.kernel.org/lkml/20230914223928.2374933-1-adrian.laru...@collabora.com/
- Removed explicit initialisation of atomic variable for profiling mode,
as it's allocated with kzalloc.
- Pass engine utilisation structure to jobs rather than the file context, to
avoid
future misusage of the latter.
- Remove double reading of cycle counter register and ktime in job deqeueue
function,
as the scheduler will make sure these values are read over in case of
requeuing.
- Moved putting of cycle counting refcnt into panfrost job dequeue.
function to avoid repetition.
v6:
https://lore.kernel.org/lkml/c73ad42b-a8db-23c2-86c7-1a2939dba...@linux.intel.com/T/
- Fix wrong swapped-round engine time and cycle values in fdinfo
drm print statements.
v7:
https://lore.kernel.org/lkml/20230927213133.1651169-6-adrian.laru...@collabora.com/T/
- Make sure an object's actual RSS size is added to the overall fdinfo's
purgeable
and active size tally when it's both resident and purgeable or active.
- Create a drm/panfrost.rst documentation file with meaning of fdinfo strings.
- BUILD_BUG_ON checking the engine name array size for fdinfo.
- Added copyright notices for Amazon in Panfrost's new debugfs files.
- Discarded fdinfo memory stats unit size selection patch.
v8:
- Style improvements and addressing nitpicks.
Adrián Larumbe (5):
drm/panfrost: Add cycle count GPU register definitions
drm/panfrost: Add fdinfo support GPU load metrics
drm/panfrost: Add fdinfo support for memory stats
drm/drm_file: Add DRM obj's RSS reporting function for fdinfo
drm/panfrost: Implement generic DRM object RSS reporting function
Documentation/gpu/drm-usage-stats.rst | 1 +
Documentation/gpu/panfrost.rst | 38 +
drivers/gpu/drm/drm_file.c | 8 +--
drivers/gpu/drm/panfrost/Makefile | 2 +
drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21
drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 +
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 8 +++
drivers/gpu/drm/panfrost/panfrost_devfreq.h | 3 ++
drivers/gpu/drm/panfrost/panfrost_device.c | 2 +
drivers/gpu/drm/panfrost/panfrost_device.h | 13 +
drivers/gpu/drm/panfrost/panfrost_drv.c | 60 -
drivers/gpu/drm/panfrost/panfrost_gem.c | 30 +++
drivers/gpu/drm/panfrost/panfrost_gem.h | 5 ++
drivers/gpu/drm/panfrost/panfrost_gpu.c | 41 ++
drivers/gpu/drm/panfrost/panfrost_gpu.h | 4 ++
drivers/gpu/drm/panfrost/panfrost_job.c | 24 +
drivers/gpu/drm/panfrost/panfrost_job.h | 5 ++
drivers/gpu/drm/panfrost/panfrost_mmu.c | 1 +
drivers/gpu/drm/panfrost/panfrost_regs.h| 5
Re: [PATCH v4 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI
Hi Luca,
Am Freitag, dem 29.09.2023 um 18:48 +0200 schrieb Luca Ceresoli:
> Hi Lucas,
>
> On Thu, 28 Sep 2023 14:55:35 +0200
> Lucas Stach wrote:
>
> > Add binding for the i.MX8MP HDMI parallel video interface block.
> >
> > Signed-off-by: Lucas Stach
> > Reviewed-by: Laurent Pinchart
> > ---
> > .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 83 +++
> > 1 file changed, 83 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> > b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> > new file mode 100644
> > index ..df29006b4090
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> > @@ -0,0 +1,83 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/imx/fsl,imx8mp-hdmi-pvi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale i.MX8MP HDMI Parallel Video Interface
> > +
> > +maintainers:
> > + - Lucas Stach
> > +
> > +description: |
> > + The HDMI parallel video interface is a timing and sync generator block
> > in the
> > + i.MX8MP SoC, that sits between the video source and the HDMI TX
> > controller.
> > +
> > +properties:
> > + compatible:
> > +const: fsl,imx8mp-hdmi-pvi
> > +
> > + reg:
> > +maxItems: 1
> > +
> > + interrupts:
> > +maxItems: 1
> > +
> > + power-domains:
> > +maxItems: 1
> > +
> > + ports:
> > +$ref: /schemas/graph.yaml#/properties/ports
> > +
> > +properties:
> > + port@0:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: Input from the LCDIF controller.
> > +
> > + port@1:
> > +$ref: /schemas/graph.yaml#/properties/port
> > +description: Output to the HDMI TX controller.
> > +
> > +required:
> > + - port@0
> > + - port@1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
>
> Sure it is required? In the imx8mp.dtsi I have, which comes for a patch
> you sent previously, there is no 'interrupts' property, and HDMI works.
>
Yes, the driver doesn't use/enforce this interrupt at the moment and
will work without it. But since the IRQ is present in the only known HW
implementation of this IP, I don't see a reason to make it optional in
the DT, as that's just proper description of the HW.
> > + - power-domains
> > + - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +#include
> > +#include
> > +
> > +display-bridge@32fc4000 {
> > +compatible = "fsl,imx8mp-hdmi-pvi";
> > +reg = <0x32fc4000 0x40>;
>
> The device has up to register 0x40, thus I guess the second value should
> be 0x44 here. Or maybe 0x100, just to be comfortable. :)
>
Right, I'll fix that.
Regards,
Lucas
Re: [PATCH] drm/msm/a6xx: don't set IO_PGTABLE_QUIRK_ARM_OUTER_WBWA with coherent SMMU
On 29/09/2023 4:45 pm, Will Deacon wrote: On Mon, Sep 25, 2023 at 06:54:42PM +0100, Robin Murphy wrote: On 2023-04-10 19:52, Dmitry Baryshkov wrote: If the Adreno SMMU is dma-coherent, allocation will fail unless we disable IO_PGTABLE_QUIRK_ARM_OUTER_WBWA. Skip setting this quirk for the coherent SMMUs (like we have on sm8350 platform). Hmm, but is it right that it should fail in the first place? The fact is that if the SMMU is coherent then walks *will* be outer-WBWA, so I honestly can't see why the io-pgtable code is going out of its way to explicitly reject a request to give them the same attribute it's already giving then anyway :/ Even if the original intent was for the quirk to have an over-specific implication of representing inner-NC as well, that hardly seems useful if what we've ended up with in practice is a nonsensical-looking check in one place and then a weird hacky bodge in another purely to work around it. Does anyone know a good reason why this is the way it is? I think it was mainly because the quick doesn't make sense for a coherent page-table walker and we could in theory use that bit for something else in that case. Yuck, even if we did want some horrible notion of quirks being conditional on parts of the config rather than just the format, then the users would need to be testing for the same condition as the pagetable code itself (i.e. cfg->coherent_walk), rather than hoping some other property of something else indirectly reflects the right information - e.g. there'd be no hope of backporting this particular bodge before 5.19 where the old iommu_capable(IOMMU_CAP_CACHE_COHERENCY) always returned true, and in future we could conceivably support coherent SMMUs being configured for non-coherent walks on a per-domain basis. Furthermore, if we did overload a flag to have multiple meanings, then we'd have no way of knowing which one the caller was actually expecting, thus the illusion of being able to validate calls in the meantime isn't necessarily as helpful as it seems, particularly in a case where the "wrong" interpretation would be to have no effect anyway. Mostly though I'd hope that if we ever got anywhere near the point of running out of quirk bits we'd have already realised that it's time for a better interface :( Based on that, I think that when I do get round to needing to touch this code, I'll propose just streamlining the whole quirk. Cheers, Robin.
Re: [PATCH v4 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI
Hi Lucas,
On Thu, 28 Sep 2023 14:55:35 +0200
Lucas Stach wrote:
> Add binding for the i.MX8MP HDMI parallel video interface block.
>
> Signed-off-by: Lucas Stach
> Reviewed-by: Laurent Pinchart
> ---
> .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 83 +++
> 1 file changed, 83 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> new file mode 100644
> index ..df29006b4090
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,imx8mp-hdmi-pvi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8MP HDMI Parallel Video Interface
> +
> +maintainers:
> + - Lucas Stach
> +
> +description: |
> + The HDMI parallel video interface is a timing and sync generator block in
> the
> + i.MX8MP SoC, that sits between the video source and the HDMI TX controller.
> +
> +properties:
> + compatible:
> +const: fsl,imx8mp-hdmi-pvi
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +maxItems: 1
> +
> + power-domains:
> +maxItems: 1
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Input from the LCDIF controller.
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Output to the HDMI TX controller.
> +
> +required:
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
Sure it is required? In the imx8mp.dtsi I have, which comes for a patch
you sent previously, there is no 'interrupts' property, and HDMI works.
> + - power-domains
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +
> +display-bridge@32fc4000 {
> +compatible = "fsl,imx8mp-hdmi-pvi";
> +reg = <0x32fc4000 0x40>;
The device has up to register 0x40, thus I guess the second value should
be 0x44 here. Or maybe 0x100, just to be comfortable. :)
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v4 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface
Hi Lucas, On Thu, 28 Sep 2023 14:55:36 +0200 Lucas Stach wrote: > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > full timing generator and can switch between different video sources. On > the i.MX8MP however the only supported source is the LCDIF. The block > just needs to be powered up and told about the polarity of the video > sync signals to act in bypass mode. > > Signed-off-by: Lucas Stach > Reviewed-by: Luca Ceresoli (v2) I was in Cc on your v3 but not on this v4. Maybe the " (v2)" on these lines confuses get_maintainers.pl? > Tested-by: Marek Vasut (v1) > Tested-by: Luca Ceresoli (v2) > Tested-by: Richard Leitner (v2) > Tested-by: Frieder Schrempf (v2) > Reviewed-by: Laurent Pinchart (v3) A changelog would be appreciated, especially as a long time has gone since I last looked at these patches. Confirmed for this v4: Reviewed-by: Luca Ceresoli [On Avnet MSC SM2-MB-EP1 based on the SMARC module] Tested-by: Luca Ceresoli Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[Bug 216806] [Raven Ridge] console disappears after framebuffer device loads
https://bugzilla.kernel.org/show_bug.cgi?id=216806 --- Comment #5 from Balazs Vinarz (viniba...@gmail.com) --- The latest 6.5.5-arch1-1 kernel just turned the display into a full white screen with high brightness. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] drm/msm/a6xx: don't set IO_PGTABLE_QUIRK_ARM_OUTER_WBWA with coherent SMMU
On Mon, Sep 25, 2023 at 06:54:42PM +0100, Robin Murphy wrote: > On 2023-04-10 19:52, Dmitry Baryshkov wrote: > > If the Adreno SMMU is dma-coherent, allocation will fail unless we > > disable IO_PGTABLE_QUIRK_ARM_OUTER_WBWA. Skip setting this quirk for the > > coherent SMMUs (like we have on sm8350 platform). > > Hmm, but is it right that it should fail in the first place? The fact is > that if the SMMU is coherent then walks *will* be outer-WBWA, so I honestly > can't see why the io-pgtable code is going out of its way to explicitly > reject a request to give them the same attribute it's already giving then > anyway :/ > > Even if the original intent was for the quirk to have an over-specific > implication of representing inner-NC as well, that hardly seems useful if > what we've ended up with in practice is a nonsensical-looking check in one > place and then a weird hacky bodge in another purely to work around it. > > Does anyone know a good reason why this is the way it is? I think it was mainly because the quick doesn't make sense for a coherent page-table walker and we could in theory use that bit for something else in that case. Will
[lvc-project] [PATCH] drm/amd/display: Fix potential null dereference
The 'top_pipe_to_program pointer' can be NULL and it is checked
at the first dereference, but not at the second.
Add a check before using it.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Igor Artemiev
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 3a9077b60029..154ad23ff931 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3824,7 +3824,8 @@ static void commit_planes_for_stream(struct dc *dc,
}
if ((update_type != UPDATE_TYPE_FAST) &&
stream->update_flags.bits.dsc_changed)
- if
(top_pipe_to_program->stream_res.tg->funcs->lock_doublebuffer_enable) {
+ if (top_pipe_to_program &&
+
top_pipe_to_program->stream_res.tg->funcs->lock_doublebuffer_enable) {
top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
top_pipe_to_program->stream_res.tg,
CRTC_STATE_VACTIVE);
--
2.30.2
Re: [PATCH v2 0/5] drm: Reuse temporary memory for format conversion
Hi Am 29.09.23 um 14:11 schrieb Maxime Ripard: On Wed, Sep 20, 2023 at 04:24:26PM +0200, Thomas Zimmermann wrote: DRM's format-conversion helpers require temporary memory. Pass the buffer from the caller and keep it allocated over several calls. Allow the caller to preallocate the buffer memory. The motivation for this patchset is the recent work on a DRM panic handler. The panic handler requires format conversion to display an error to the screen. But allocating memory during kernel panics is fragile. The changes in this patchset enable the DRM panic handler to preallocate buffer storage before the panic occurs. As an additonal benefit, drivers can now keep the temporary storage across multiple display updates. Avoiding memory allocation reduces the CPU overhead of the format helpers. This argument is getting a bit tiring. The main reason is that it isn't one, and: CPU overhead isn't the driver behind this patchset, but if it is affected, why not say that in the commit message? There's a alloc/free pair for each updated scanline. For a full-screen updates, that's quite a bit. - we allocate something in the 10-20 range objects at a given commit, so another small one is not insignificant. - If it was, it would be trivial to produce a benchmark, but no-one ever actually showed a workload and numbers where there's actually any difference. - Also, the CPU overhead is indeed (even if marginally) decreased, but the memory overhead is increased. I don't know whether that's a good trade-off or not, see the point above. It really sounds like an empty statement to me: "But just think of the CPU!". That being said, I also have more fundamental doubts about this series. The first one is that storing the buffer pointer in the device instead of the state makes it harder to reason about. When you have a state, the framework provides the guarantee at commit time that there's only going to be one at a given time. And since the buffer is stored in that state object, you can't access it by mistake. The buffer size also depends on the state, so that all makes sense from a logical PoV. Yes. I discussed this with Javier already. Putting this into the state is the clean solution. If we store the buffer into the device, then suddenly you have to think about whether there's multiple CRTCs or not (since commits aren't serialised if they affect different CRTCs), whether the buffer size you allocated is large enough now for your current resolution and format, etc. It adds a decent chunk of complexity on something that was quite complex already. It's in the device because it's good enough for these simple drivers. The next best place would be a dedicated plane structure in each driver. The per-plane cache would then be clearly attributed to a single plane. I understand that the motivation is for drm_panic to have a buffer ready when it has to kick in. But why don't we just allocate (and check) a spare commit at probe time so we just have to commit it when we panic. DRM panic doesn't commit anything. It picks up whatever the current scanout buffer is and draws into that. If the DRM driver cannot provide a scanout buffer, DRM panic does nothing. Best regards Thomas That would fall nicely into the rest of the atomic modesetting code, and since we pretty much require not to allocate anything during atomic_commit, we have that constraints already figured out. Maxime -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI
On Thu, Sep 28, 2023 at 02:55:35PM +0200, Lucas Stach wrote:
> Add binding for the i.MX8MP HDMI parallel video interface block.
>
> Signed-off-by: Lucas Stach
> Reviewed-by: Laurent Pinchart
> ---
> .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 83 +++
> 1 file changed, 83 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> new file mode 100644
> index ..df29006b4090
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,imx8mp-hdmi-pvi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8MP HDMI Parallel Video Interface
> +
> +maintainers:
> + - Lucas Stach
> +
> +description: |
This | is not needed as there's no formatting here requiring
preservation.
Otherwise, looks grand to me. You can fix that up if you resend I guess.
Reviewed-by: Conor Dooley
Thanks,
Conor.
> + The HDMI parallel video interface is a timing and sync generator block in
> the
> + i.MX8MP SoC, that sits between the video source and the HDMI TX controller.
> +
> +properties:
> + compatible:
> +const: fsl,imx8mp-hdmi-pvi
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +maxItems: 1
> +
> + power-domains:
> +maxItems: 1
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Input from the LCDIF controller.
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Output to the HDMI TX controller.
> +
> +required:
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - power-domains
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +
> +display-bridge@32fc4000 {
> +compatible = "fsl,imx8mp-hdmi-pvi";
> +reg = <0x32fc4000 0x40>;
> +interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
> +power-domains = <_blk_ctrl IMX8MP_HDMIBLK_PD_PVI>;
> +
> +ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +port@0 {
> +reg = <0>;
> +pvi_from_lcdif3: endpoint {
> +remote-endpoint = <_to_pvi>;
> +};
> +};
> +
> +port@1 {
> +reg = <1>;
> +pvi_to_hdmi_tx: endpoint {
> +remote-endpoint = <_tx_from_pvi>;
> +};
> +};
> +};
> +};
> --
> 2.39.2
>
signature.asc
Description: PGP signature
Re: [PATCH v3 0/4] Fix up the boe-tv101wum-nl6 panel driver
Hi, On Thu, Sep 28, 2023 at 2:42 PM Linus Walleij wrote: > > On Tue, Sep 26, 2023 at 11:49 PM Doug Anderson wrote: > > > > I'm curious what the latest on this patch series is. Is it abandoned, > > > or is it still on your list to move forward with it? If it's > > > abandoned, does that mean we've abandoned the idea of breaking > > > ili9882t into a separate driver? > > > > > > From looking at things that have landed downstream in the ChromeOS > > > kernel trees it looks as if additional fixes are getting blocked from > > > being posted/landed because of the limbo state that this is in. > > > > I presume Linus is busy or otherwise indisposed. > > Sorry I was looking for the branch with my patches and I have it > somewhere not ordinary :/ > > Originally I shelved it because I got requests to do additional > patches to the driver: > https://lore.kernel.org/dri-devel/CAD=FV=xkr3qpd8m_6xta_2jl_ezbxsmmyarbkxtxl+ujlg9...@mail.gmail.com/ > > To do measurements about binary code size in object files, and if it does, > then I need to invent new sequence macros (IIUC): > https://lore.kernel.org/dri-devel/CAD=FV=Wju3WS45=EpXMUg7FjYDh3-=mvm_js7tf1tsaazbb...@mail.gmail.com/ > > So I just didn't have time for that extensive rework of the driver. > > It's good feedback, but I just wanted to make the situation a little > bit better, and perfect is the enemy of good (TM). > > > So I guess we have two options here: > > > > a) Cong Yang can post any relevant fixes to the existing "monolithic" > > panel driver so that we can get them landed and at least get things > > fixed. > > > > - or - > > > > b) Cong Yang could take over all or some of Linus's series and post > > new versions of it, addressing feedback. > > Either works for me, I would prefer b), Cong is welcome to adopt > the patches if he/his employer want to. Go ahead! > > We can't really let this one-size-fits-all driver go on like this. > > My main concern with the "boe-tv101wum-nl6" driver is that it can > be renamed "cromeos-hackfest" at this point because it becomes > hard for any other system to reuse the panel drivers, the typical > example would be a system using say ili9882t but with > a different init sequence or something, why would they want > support for 9 unrelated panels compiled in? The condition that > these drivers should be related to the original panel that gave > name to the file has seemingly been dropped long ago. > > It looks like the drivers only share the power lines (avdd, avee, pp3300, > pp1800) then this can be broken out to a helper library. But I am > sceptical about that too. I doubt that the vastly different panels > actually have exactly these these supply line names, I think it is > actually names of the rails on the chrome machine board. And that is > not how these regulators should be named, they should be named after > the input name on the component. This is really hard to catch in reviews when > we don't have datasheets so I'm not blaming anyone, but is this something > that even needs fixing in the device tree bindings? (By deprecation > and addition...) can we look into this? > > I would say can we at least agree that before we merge one more > driver into this file, break out to subdrivers those that clearly have > an identifiable display controller and is thus reusable? From my > point of view I can just see the ili9882t so that's a good start. This sounds like a reasonable plan to me. What if Cong posted patches that broke this up into a separate driver for the distinct controller but otherwise didn't substantially reorganize it? In other words both the old driver and the new one would keep the "struct panel_init_cmd" until we get some resolution about the binary size issue. That would at least let us move forward... -Doug
Re: [PATCH 14/15] hyper-v/azure: Remove now superfluous sentinel element from ctl_table array
On Thu, Sep 28, 2023 at 03:26:16PM +, Wei Liu wrote:
> Please change the prefix to "Drivers: hv:" in the subject line in the
> two patches.
I'll change the commit message for the 14/15 patch from "hyper-v/azure"
to "Drivers: hv:". But I only see one patch that needs this. Which is
the other one?
best
>
> On Thu, Sep 28, 2023 at 03:21:39PM +0200, Joel Granados via B4 Relay wrote:
> > From: Joel Granados
> >
> > This commit comes at the tail end of a greater effort to remove the
> > empty elements at the end of the ctl_table arrays (sentinels) which
> > will reduce the overall build time size of the kernel and run time
> > memory bloat by ~64 bytes per sentinel (further information Link :
> > https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
> >
> > Remove sentinel from hv_ctl_table
> >
> > Signed-off-by: Joel Granados
> > ---
> > drivers/hv/hv_common.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> > index ccad7bca3fd3..bc7d678030aa 100644
> > --- a/drivers/hv/hv_common.c
> > +++ b/drivers/hv/hv_common.c
> > @@ -147,8 +147,7 @@ static struct ctl_table hv_ctl_table[] = {
> > .proc_handler = proc_dointvec_minmax,
> > .extra1 = SYSCTL_ZERO,
> > .extra2 = SYSCTL_ONE
> > - },
> > - {}
> > + }
>
> Please keep the comma at the end.
>
> > };
> >
> > static int hv_die_panic_notify_crash(struct notifier_block *self,
> >
> > --
> > 2.30.2
> >
--
Joel Granados
signature.asc
Description: PGP signature
Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195
On Fri, Sep 29, 2023 at 10:42:58AM +0200, AngeloGioacchino Del Regno wrote:
> Il 28/09/23 18:49, Conor Dooley ha scritto:
> > On Thu, Sep 28, 2023 at 02:52:23AM +, Moudy Ho (何宗原) wrote:
> > > On Wed, 2023-09-27 at 10:47 +0100, Conor Dooley wrote:
> > > > On Wed, Sep 27, 2023 at 07:19:28AM +, Moudy Ho (何宗原) wrote:
> > > > > On Fri, 2023-09-22 at 16:51 +0100, Conor Dooley wrote:
> > > > > > On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
> > > > > > > On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
> > > > > > > > Add a compatible string for the COLOR block in MediaTek
> > > > > > > > MT8195
> > > > > > > > that
> > > > > > > > is controlled by MDP3.
> > > > > > > >
> > > > > > > > Signed-off-by: Moudy Ho
> > > > > > > > ---
> > > > > > > > .../devicetree/bindings/display/mediatek/mediatek,color.yaml
> > > > > > > > | 1 +
> > > > > > > > 1 file changed, 1 insertion(+)
> > > > > > > >
> > > > > > > > diff --git
> > > > > > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > > > ,col
> > > > > > > > or.yaml
> > > > > > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > > > ,col
> > > > > > > > or.yaml
> > > > > > > > index f21e44092043..b886ca0d89ea 100644
> > > > > > > > ---
> > > > > > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > > > ,col
> > > > > > > > or.yaml
> > > > > > > > +++
> > > > > > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > > > ,col
> > > > > > > > or.yaml
> > > > > > > > @@ -26,6 +26,7 @@ properties:
> > > > > > > > - mediatek,mt2701-disp-color
> > > > > > > > - mediatek,mt8167-disp-color
> > > > > > > > - mediatek,mt8173-disp-color
> > > > > > > > + - mediatek,mt8195-mdp3-color
> > > > > > >
> > > > > > > How come this one is a "mdp3" not a "disp"?
> > > > > >
> > > > > > I don't know what mdp3 means & googling gives me no answers.
> > > > > > What's
> > > > > > the
> > > > > > "disp" one controlled by, since it isn't controlled by mdp3?
> > > > > >
> >
> > > > > Mediatek's Media Data Path ver.3 (MDP3) is associated with MMSYS
> > > > > and
> > > > > acts as an independent driver that operates between VDEC and DISP.
> > > > > By controlling multiple components, it carries out tasks like
> > > > > converting color formats, resizing, and applying specific Picture
> > > > > Quality (PQ) effects.
> > > > > The driver can be found at "driver/media/platform/mediatek/mdp3".
> > > > > Since the same hardware components are configured in both MDP3 and
> > > > > DISP, considering previous discussions, I attemped to integrate
> > > > > into a
> > > > > single binding, named after the controlling user.
> > > >
> > > > I'm still kinda struggling to understand this. Do you mean that the
> > > > hardware can be controlled by either of the disp and mdp3 drivers,
> > > > and
> > > > a compatible containing "disp" would use one driver, and one
> > > > containing
> > > > "mdp3" would use another?
> > > >
> >
> > > Sorry for any confusion caused by the software information. In the
> > > video pipeline, after decoding, the data flows sequentially through two
> > > subsystems: MDP and DISP. Each subsystems has multiple IPs, with some
> > > serving the same functionality as COLOR mentioned here. However, these
> > > IPs cannot be controlled by different subsystems. Therefore, I included
> > > the name of the subsystem after SoC to identify the configuration's
> > > location. Is this approach feasible?
> >
> > I'll have to leave things to the likes of Laurent to comment here I
> > think. I don't understand this hardware well enough to have a useful
> > opinion. It would seem like a different part of the datapath is a
> > different device that should be documented separately, but I don't know
> > enough to say for sure, sorry.
>
> Hardware speaking, it's not a different device: those all reside in the
> same block, except they are configured to route their I/O *either* to the
> display pipeline, *or* to the MDP3 pipeline.
Is it runtime configurable?
> I would agree though in that this could be more flexible, as in, not
> having a requirement to say "mdp3" or "disp", and managing the COLOR
> blocks generically and letting the drivers to choose the actual path
> transparently from what the devicetree compatible is, but there's no
> practical point in doing this in the end, because there is an enough
> number of (for example, COLOR) blocks such that one can be completely
> reserved to MDP3 and one completely reserved to DISP.
>
> So, we don't *need* this flexibility, but would be nice to have for
> different (unexistant, basically) usecases...
>
> The thing is, if we go for the maximum flexibility, the drawback is
> that we'd see a number of nodes like
>
> shared_block: something@somewhere {
> compatible = "mediatek,something";
> }
>
> mdp3: dma-controller@14001000 {
> ..
>
Re: [PATCH v14 RESEND 1/6] dt-bindings: display: imx: Add i.MX8qxp/qm DPU binding
On Tue, Sep 05, 2023 at 03:44:23AM +, Ying Liu wrote: > On Thursday, August 24, 2023 5:48 PM, Maxime Ripard > wrote: > > On Wed, Aug 23, 2023 at 08:47:51AM +, Ying Liu wrote: > > > > > This dt-binding just follows generic dt-binding rule to describe the > > > > > DPU > > IP > > > > > hardware, not the software implementation. DPU internal units do not > > > > > constitute separate devices. > > > > > > > > I mean, your driver does split them into separate devices so surely it > > > > constitutes separate devices. > > > > > > My driver treats them as DPU internal units, especially not Linux devices. > > > > > > Let's avoid Linuxisms when implementing this dt-binding and just be simple > > > to describe necessary stuff exposing to DPU's embodying system/SoC, like > > > reg, interrupts, clocks and power-domains. > > > > Let's focus the conversation here, because it's redundant with the rest. > > > > Your driver registers two additional devices, that have a different > > register space, different clocks, different interrupts, different power > > domains, etc. That has nothing to do with Linux, it's hardware > > properties. > > > > That alone is a very good indication to me that these devices should be > > modeled as such. And your driver agrees. > > > > Whether or not the other internal units need to be described as separate > > devices, I can't really tell, I don't have the datasheet. > > i.MX8qxp and i.MX8qm SoC reference manuals can be found at(I think > registration is needed first): > https://www.nxp.com/webapp/Download?colCode=IMX8DQXPRM > https://www.nxp.com/webapp/Download?colCode=IMX8QMRM I tried, but the registration is buggy. The email takes longer than the timeout to be sent. > Sorry for putting this in a short way, but the DPU is one IP, so one > dt-binding. > > > > > But at least the CRTC and the interrupt controller should be split away, > > or explained and detailed far better than "well it's just convenient". > > CRTC is Linuxisms, which cannot be referenced to determine dt-binding. > > DPU as Display Controller is listed as a standalone module/IP in RM. > This is how the IP is designed in the first place, not for any convenient > purpose. Sure, but pushing that argument further, the entire SoC has been designed as a single entity. Every vendor out there designs its display pipeline in its entirety and not block by block. This doesn't mean that it isn't composed of several mostly discrete components. If it has a separate address space, clock and interrupt, it's a different device, no matter how it was designed or what the intent was. Maxime signature.asc Description: PGP signature
Re: [REGRESSION] Panic in gen8_ggtt_insert_entries() with v6.5
On 19.09.23 16:08, Bagas Sanjaya wrote: > On Sat, Sep 02, 2023 at 06:14:12PM +0200, Oleksandr Natalenko wrote: >> >> Since v6.5 kernel the following HW: >> >> * Lenovo T460s laptop with Skylake GT2 [HD Graphics 520] (rev 07) >> * Lenovo T490s laptop with WhiskeyLake-U GT2 [UHD Graphics 620] (rev 02) > > #regzbot ^introduced: 0b62af28f249b9 > #regzbot title: gen8_ggtt_insert_entries() panic on Lenovo T14s (Tiger Lake) > due to folio_batch() on shmem_sg_free_table() > #regzbot link: https://gitlab.freedesktop.org/drm/intel/-/issues/9256 #regzbot fix: i915: Limit the length of an sg list to the requested length #regzbot ignore-activity Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you.
Re: [syzbot] upstream boot error: can't ssh into the instance (15)
On Fri, Sep 29, 2023 at 3:29 PM syzbot wrote: > > Hello, > > syzbot found the following issue on: > > HEAD commit:9ed22ae6be81 Merge tag 'spi-fix-v6.6-rc3' of git://git.ker.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=14b37a7c68 > kernel config: https://syzkaller.appspot.com/x/.config?x=d4bdf71ec9aec6cc > dashboard link: https://syzkaller.appspot.com/bug?extid=be9661ba81a9c1cf6b15 > compiler: aarch64-linux-gnu-gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU > Binutils for Debian) 2.40 > userspace arch: arm64 > > Downloadable assets: > disk image (non-bootable): > https://storage.googleapis.com/syzbot-assets/384ffdcca292/non_bootable_disk-9ed22ae6.raw.xz > vmlinux: > https://storage.googleapis.com/syzbot-assets/2c3d5eea45bd/vmlinux-9ed22ae6.xz > kernel image: > https://storage.googleapis.com/syzbot-assets/5f361432/Image-9ed22ae6.gz.xz This appears on qemu-system-aarch64 with virt,virtualization=on,mte=on,graphics=on,usb=on. I've run it locally using the assets above and it seems there are actually two problems behind the report. 1) For some reason, v7.2 of qemu-system-aarch64 just hangs with "-smp 2" and prints no output. Interestingly, it all works fine on qemu v8.0.4, so I don't know if it's a qemu or a kernel problem. Qemu v8 is unfortunately still too new for many distributions (we use debian:bookworm on syzbot and v7.2 is the latest there). 2) If I set "-smp 1", it begins to boot, but quickly fails with several messages. First with [0.00][T0] == [0.00][T0] BUG: KASAN: slab-out-of-bounds in __kasan_slab_alloc+0x7c/0xcc [0.00][T0] Read at addr fcff02c01008 by task swapper/0 [0.00][T0] Pointer tag: [fc], memory tag: [f5] [0.00][T0] [0.00][T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc3-syzkaller-00055-g9ed22ae6be81 #0 [0.00][T0] Hardware name: linux,dummy-virt (DT) [0.00][T0] Call trace: [0.00][T0] dump_backtrace+0x94/0xec [0.00][T0] show_stack+0x18/0x24 [0.00][T0] dump_stack_lvl+0x48/0x60 [0.00][T0] print_report+0x108/0x618 [0.00][T0] kasan_report+0x88/0xac [0.00][T0] __do_kernel_fault+0x17c/0x1e8 [0.00][T0] do_tag_check_fault+0x78/0x8c [0.00][T0] do_mem_abort+0x44/0x94 [0.00][T0] el1_abort+0x40/0x60 [0.00][T0] el1h_64_sync_handler+0xd8/0xe4 [0.00][T0] el1h_64_sync+0x64/0x68 [0.00][T0] __kasan_slab_alloc+0x7c/0xcc [0.00][T0] kmem_cache_alloc+0x144/0x290 [0.00][T0] bootstrap+0x2c/0x174 [0.00][T0] kmem_cache_init+0x144/0x1c8 [0.00][T0] mm_core_init+0x240/0x2d4 [0.00][T0] start_kernel+0x220/0x5fc [0.00][T0] __primary_switched+0xb4/0xbc [0.00][T0] [0.00][T0] Allocated by task 0: [0.00][T0] kasan_save_stack+0x3c/0x64 [0.00][T0] save_stack_info+0x38/0x118 [0.00][T0] kasan_save_alloc_info+0x14/0x20 [0.00][T0] __kasan_slab_alloc+0x94/0xcc [0.00][T0] kmem_cache_alloc+0x144/0x290 [0.00][T0] bootstrap+0x2c/0x174 [0.00][T0] kmem_cache_init+0x134/0x1c8 [0.00][T0] mm_core_init+0x240/0x2d4 [0.00][T0] start_kernel+0x220/0x5fc [0.00][T0] __primary_switched+0xb4/0xbc [0.00][T0] [0.00][T0] The buggy address belongs to the object at 02c01000 [0.00][T0] which belongs to the cache kmem_cache of size 208 [0.00][T0] The buggy address is located 8 bytes inside of [0.00][T0] 208-byte region [02c01000, 02c010d0) [0.00][T0] [0.00][T0] The buggy address belongs to the physical page: [0.00][T0] page:(ptrval) refcount:1 mapcount:0 mapping: index:0x0 pfn:0x42c01 [0.00][T0] flags: 0x1ffc800(slab|node=0|zone=0|lastcpupid=0x7ff|kasantag=0x0) [0.00][T0] page_type: 0x() [0.00][T0] raw: 01ffc800 fcff02c01000 dead0100 dead0122 [0.00][T0] raw: 80100010 0001 [0.00][T0] page dumped because: kasan: bad access detected [0.00][T0] [0.00][T0] Memory state around the buggy address: [0.00][T0] 02c00e00: f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 [0.00][T0] 02c00f00: f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 f0 [0.00][T0] >02c01000: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 [0.00][T0]^ [0.00][T0] 02c01100: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 [
Re: [PATCH v3 07/32] drm/amd/display: document AMDGPU pre-defined transfer functions
On 2023-09-29 03:35, Pekka Paalanen wrote:
> On Thu, 28 Sep 2023 16:16:57 -0400
> Harry Wentland wrote:
>
>> On 2023-09-25 15:49, Melissa Wen wrote:
>>> Brief documentation about pre-defined transfer function usage on AMD
>>> display driver and standardized EOTFs and inverse EOTFs.
>>>
>>> v3:
>>> - Document BT709 OETF (Pekka)
>>> - Fix description of sRGB and pure power funcs (Pekka)
>>>
>>> Co-developed-by: Harry Wentland
>>> Signed-off-by: Harry Wentland
>>> Signed-off-by: Melissa Wen
>>> ---
>>> .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 39 +++
>>> 1 file changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>>> index d03bdb010e8b..14f9c02539c6 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>>> @@ -85,6 +85,45 @@ void amdgpu_dm_init_color_mod(void)
>>> }
>>>
>>> #ifdef AMD_PRIVATE_COLOR
>>> +/* Pre-defined Transfer Functions (TF)
>>> + *
>>> + * AMD driver supports pre-defined mathematical functions for transferring
>>> + * between encoded values and optical/linear space. Depending on HW color
>>> caps,
>>> + * ROMs and curves built by the AMD color module support these transforms.
>>> + *
>>> + * The driver-specific color implementation exposes properties for
>>> pre-blending
>>> + * degamma TF, shaper TF (before 3D LUT), and blend(dpp.ogam) TF and
>>> + * post-blending regamma (mpc.ogam) TF. However, only pre-blending degamma
>>> + * supports ROM curves. AMD color module uses pre-defined coefficients to
>>> build
>>> + * curves for the other blocks. What can be done by each color block is
>>> + * described by struct dpp_color_capsand struct mpc_color_caps.
>>> + *
>>> + * AMD driver-specific color API exposes the following pre-defined transfer
>>> + * functions:
>>> + *
>>> + * - Linear/Unity: linear/identity relationship between pixel value and
>>> + * luminance value;
>>> + * - Gamma 2.2, Gamma 2.4, Gamma 2.6: pure power functions;
>>> + * - sRGB: 2.4: The piece-wise transfer function from IEC 61966-2-1:1999;
>>> + * - BT.709: has a linear segment in the bottom part and then a power
>>> function
>>> + * with a 0.45 (~1/2.22) gamma for the rest of the range; standardized by
>>> + * ITU-R BT.709-6;
>>> + * - PQ (Perceptual Quantizer): used for HDR display, allows luminance
>>> range
>>> + * capability of 0 to 10,000 nits; standardized by SMPTE ST 2084.
>>> + *
>>
>> I think it's important to highlight that the AMD color model is
>> designed with an assumption that SDR (sRGB, BT.709, G2.2, etc.)
>> peak white maps (normalized to 1.0 FP) to 80 nits in the PQ system.
>> This has the implication that PQ EOTF (NL-to-L) maps to [0.0..125.0].
>> 125.0 = 10,000 nits / 80 nits
>>
>> I think we'll want table or some other way describing this:
>>
>> (Using L to mean linear and NL to mean non-linear.)
>>
>> == sRGB, BT709, Gamma 2.x ==
>> NL form is either UNORM or [0.0, 1.0]
>> L form is [0.0, 1.0]
>>
>> Note that HDR multiplier can wide range beyond [0.0, 1.0].
>> In practice this means that PQ TF is needed for any subsequent
>> L-to-NL transforms.
>>
>> == PQ ==
>> NL form is either UNORM or FP16 CCCS (Windows canonical composition color
>> space, see [1])
>> L form is [0.0, 125.0]
>
> Hi,
>
> what is [1]?
>
Oops.
[1]
https://learn.microsoft.com/en-us/windows/win32/direct3darticles/high-dynamic-range
Harry
>
> Thanks,
> pq
>
>> == Unity, Default ==
>> NL form is either UNORM or FP16 CCCS
>> L form is either [0.0, 1.0] (mapping from UNORM) or CCCS (mapping from CCCS
>> FP16)
>>
>> Harry
>>
>>> + * In the driver-specific API, color block names attached to TF properties
>>> + * suggest the intention regarding non-linear encoding pixel's luminance
>>> + * values. As some newer encodings don't use gamma curve, we make encoding
>>> and
>>> + * decoding explicit by defining an enum list of transfer functions
>>> supported
>>> + * in terms of EOTF and inverse EOTF, where:
>>> + *
>>> + * - EOTF (electro-optical transfer function): is the transfer function to
>>> go
>>> + * from the encoded value to an optical (linear) value. De-gamma
>>> functions
>>> + * traditionally do this.
>>> + * - Inverse EOTF (simply the inverse of the EOTF): is usually intended to
>>> go
>>> + * from an optical/linear space (which might have been used for blending)
>>> + * back to the encoded values. Gamma functions traditionally do this.
>>> + */
>>> static const char * const
>>> amdgpu_transfer_function_names[] = {
>>> [AMDGPU_TRANSFER_FUNCTION_DEFAULT] = "Default",
>>
>>
>
[PATCH] drm/bridge: adv7511: Convert to use maple tree register cache
The maple tree register cache is based on a much more modern data structure
than the rbtree cache and makes optimisation choices which are probably
more appropriate for modern systems than those made by the rbtree cache.
Signed-off-by: Mark Brown
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 2611afd2c1c1..d518de88b5c3 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -121,7 +121,7 @@ static const struct regmap_config adv7511_regmap_config = {
.val_bits = 8,
.max_register = 0xff,
- .cache_type = REGCACHE_RBTREE,
+ .cache_type = REGCACHE_MAPLE,
.reg_defaults_raw = adv7511_register_defaults,
.num_reg_defaults_raw = ARRAY_SIZE(adv7511_register_defaults),
@@ -1068,7 +1068,7 @@ static const struct regmap_config
adv7511_cec_regmap_config = {
.val_bits = 8,
.max_register = 0xff,
- .cache_type = REGCACHE_RBTREE,
+ .cache_type = REGCACHE_MAPLE,
.volatile_reg = adv7511_cec_register_volatile,
};
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230929-drm-adv7511-2d592921f8a2
Best regards,
--
Mark Brown
[PATCH v2 2/2] drm/panel: Add driver for BOE RM692E5 AMOLED panel
Add support for the 2700x1224 AMOLED BOE panel bundled with a RM692E5
driver IC, as found on the Fairphone 5 smartphone.
Co-developed-by: Luca Weiss
Signed-off-by: Luca Weiss
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 423 ++
3 files changed, 433 insertions(+)
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 2d6d96ee3547..ecb22ea326cb 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -516,6 +516,15 @@ config DRM_PANEL_RAYDIUM_RM68200
Say Y here if you want to enable support for Raydium RM68200
720x1280 DSI video mode panel.
+config DRM_PANEL_RAYDIUM_RM692E5
+ tristate "Raydium RM692E5-based DSI panel"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y here if you want to enable support for Raydium RM692E5-based
+ display panels, such as the one found in the Fairphone 5 smartphone.
+
config DRM_PANEL_RONBO_RB070D30
tristate "Ronbo Electronics RB070D30 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 157c77ff157f..e14ce55a0875 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) +=
panel-panasonic-vvx10f034n00.o
obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) +=
panel-raspberrypi-touchscreen.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
+obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_ATNA33XC20) += panel-samsung-atna33xc20.o
obj-$(CONFIG_DRM_PANEL_SAMSUNG_DB7430) += panel-samsung-db7430.o
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
new file mode 100644
index ..a613ba5b816c
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device
tree.
+ * Copyright (c) 2023 Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+struct rm692e5_panel {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct drm_dsc_config dsc;
+ struct regulator_bulk_data supplies[3];
+ struct gpio_desc *reset_gpio;
+ bool prepared;
+};
+
+static inline struct rm692e5_panel *to_rm692e5_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct rm692e5_panel, panel);
+}
+
+static void rm692e5_reset(struct rm692e5_panel *ctx)
+{
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(1, 11000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+ usleep_range(5000, 6000);
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ usleep_range(1, 11000);
+}
+
+static int rm692e5_on(struct rm692e5_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi;
+ struct device *dev = >dev;
+ int ret;
+
+ dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+ mipi_dsi_generic_write_seq(dsi, 0xfe, 0x41);
+ mipi_dsi_generic_write_seq(dsi, 0xd6, 0x00);
+ mipi_dsi_generic_write_seq(dsi, 0xfe, 0x16);
+ mipi_dsi_generic_write_seq(dsi, 0x8a, 0x87);
+ mipi_dsi_generic_write_seq(dsi, 0xfe, 0x71);
+ mipi_dsi_generic_write_seq(dsi, 0x82, 0x01);
+ mipi_dsi_generic_write_seq(dsi, 0xc6, 0x00);
+ mipi_dsi_generic_write_seq(dsi, 0xc7, 0x2c);
+ mipi_dsi_generic_write_seq(dsi, 0xc8, 0x64);
+ mipi_dsi_generic_write_seq(dsi, 0xc9, 0x3c);
+ mipi_dsi_generic_write_seq(dsi, 0xca, 0x80);
+ mipi_dsi_generic_write_seq(dsi, 0xcb, 0x02);
+ mipi_dsi_generic_write_seq(dsi, 0xcc, 0x02);
+ mipi_dsi_generic_write_seq(dsi, 0xfe, 0x38);
+ mipi_dsi_generic_write_seq(dsi, 0x18, 0x13);
+ mipi_dsi_generic_write_seq(dsi, 0xfe, 0xf4);
+ mipi_dsi_generic_write_seq(dsi, 0x00, 0xff);
+ mipi_dsi_generic_write_seq(dsi, 0x01, 0xff);
+ mipi_dsi_generic_write_seq(dsi, 0x02, 0xcf);
+ mipi_dsi_generic_write_seq(dsi, 0x03, 0xbc);
+ mipi_dsi_generic_write_seq(dsi, 0x04, 0xb9);
+ mipi_dsi_generic_write_seq(dsi, 0x05, 0x99);
+ mipi_dsi_generic_write_seq(dsi, 0x06, 0x02);
+ mipi_dsi_generic_write_seq(dsi, 0x07, 0x0a);
+ mipi_dsi_generic_write_seq(dsi, 0x08, 0xe0);
+ mipi_dsi_generic_write_seq(dsi, 0x09, 0x4c);
+ mipi_dsi_generic_write_seq(dsi, 0x0a, 0xeb);
+ mipi_dsi_generic_write_seq(dsi, 0x0b, 0xe8);
+
[PATCH v2 1/2] dt-bindings: display: panel: Add Raydium RM692E5
Raydium RM692E5 is a display driver IC used to drive AMOLED DSI panels.
Describe it.
Reviewed-by: Conor Dooley
Signed-off-by: Konrad Dybcio
---
.../bindings/display/panel/raydium,rm692e5.yaml| 73 ++
1 file changed, 73 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/panel/raydium,rm692e5.yaml
b/Documentation/devicetree/bindings/display/panel/raydium,rm692e5.yaml
new file mode 100644
index ..f436ba6738ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/raydium,rm692e5.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/raydium,rm692e5.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raydium RM692E5 based DSI display panels
+
+maintainers:
+ - Konrad Dybcio
+
+description:
+ The Raydium RM692E5 is a generic DSI Panel IC used to control
+ AMOLED panels.
+
+allOf:
+ - $ref: panel-common.yaml#
+
+properties:
+ compatible:
+items:
+ - const: fairphone,fp5-rm692e5-boe
+ - const: raydium,rm692e5
+
+ dvdd-supply:
+description: Digital voltage rail
+
+ vci-supply:
+description: Analog voltage rail
+
+ vddio-supply:
+description: I/O voltage rail
+
+ reg: true
+ port: true
+
+required:
+ - compatible
+ - reg
+ - reset-gpios
+ - dvdd-supply
+ - vci-supply
+ - vddio-supply
+ - port
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "fairphone,fp5-rm692e5-boe", "raydium,rm692e5";
+reg = <0>;
+
+reset-gpios = < 44 GPIO_ACTIVE_LOW>;
+dvdd-supply = <_oled_vci>;
+vci-supply = <_l12c>;
+vddio-supply = <_oled_dvdd>;
+
+port {
+panel_in_0: endpoint {
+remote-endpoint = <_out>;
+};
+};
+};
+};
+
+...
--
2.42.0
[PATCH v2 0/2] Raydium RM692E5-based BOE panel driver
The Fairphone 5 smartphone ships with a BOE AMOLED panel in conjunction with a Raydium RM692E5 driver IC. This series adds the bindings and driver for that. Signed-off-by: Konrad Dybcio --- Changes in v2: DRIVER: - Remove 1ms sleeps after each DCS command submission - Remove WARN_ON from probe() - Move DCS off commands from .unprepare to .disable so that they actually reach the DSI host - Do NOT introduce the 60 Hz mode for now, exploring ways to avoid tons of boilerplate BINDINGS: - Fix weird capitalization at the end of a sentence in title: - Drop unnecessary | in description: - pick up rb - Link to v1: https://lore.kernel.org/r/20230927-topic-fp5_disp-v1-0-a6aabd681...@linaro.org --- Konrad Dybcio (2): dt-bindings: display: panel: Add Raydium RM692E5 drm/panel: Add driver for BOE RM692E5 AMOLED panel .../bindings/display/panel/raydium,rm692e5.yaml| 73 drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 423 + 4 files changed, 506 insertions(+) --- base-commit: df964ce9ef9fea10cf131bf6bad8658fde7956f6 change-id: 20230927-topic-fp5_disp-7671c009be18 Best regards, -- Konrad Dybcio
[PATCH v5 3/3] drm/panel-simple: allow LVDS format override
Some panels support multiple LVDS data mapping formats, which can be
used e.g. run displays on jeida-18 format when only 3 LVDS lanes are
available.
Add parsing of an optional data-mapping devicetree property, which also
touches up the bits per color to match the bus format.
Signed-off-by: Johannes Zink
---
Changes:
v4 -> v5: none
v3 -> v4: - worked in Dan's feedback (thanks for reviewing my work):
- return with a proper error in case the call to
panel_simple_override_nondefault_lvds_datamapping()
fails
- drop the unneeded and ambiguous ret variable
v2 -> v3: - worked in Laurent's review findings (thanks for reviewing
my work):
- extract fixing up the bus format to separate
function
- only call function on LVDS panels
- fix typos found by Laurent
- simplified error handling
v1 -> v2: - fix missing unwind goto found by test robot
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Link:
https://lore.kernel.org/r/202304160359.4lhmfolu-...@intel.com/
---
drivers/gpu/drm/panel/panel-simple.c | 53
1 file changed, 53 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 857bc01591db..4195cf54934b 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
/**
* struct panel_desc - Describes a simple panel.
@@ -549,6 +550,51 @@ static void panel_simple_parse_panel_timing_node(struct
device *dev,
dev_err(dev, "Reject override mode: No display_timing found\n");
}
+static int panel_simple_override_nondefault_lvds_datamapping(struct device
*dev,
+struct
panel_simple *panel)
+{
+ int ret, bpc;
+
+ ret = drm_of_lvds_get_data_mapping(dev->of_node);
+ if (ret < 0) {
+ if (ret == -EINVAL)
+ dev_warn(dev, "Ignore invalid data-mapping property\n");
+
+ /*
+* Ignore non-existing or malformatted property, fallback to
+* default data-mapping, and return 0.
+*/
+ return 0;
+ }
+
+ switch (ret) {
+ default:
+ WARN_ON(1);
+ fallthrough;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+ fallthrough;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+ bpc = 8;
+ break;
+ case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+ bpc = 6;
+ }
+
+ if (panel->desc->bpc != bpc || panel->desc->bus_format != ret) {
+ struct panel_desc *override_desc;
+
+ override_desc = devm_kmemdup(dev, panel->desc,
sizeof(*panel->desc), GFP_KERNEL);
+ if (!override_desc)
+ return -ENOMEM;
+
+ override_desc->bus_format = ret;
+ override_desc->bpc = bpc;
+ panel->desc = override_desc;
+ }
+
+ return 0;
+}
+
static int panel_simple_probe(struct device *dev, const struct panel_desc
*desc)
{
struct panel_simple *panel;
@@ -601,6 +647,13 @@ static int panel_simple_probe(struct device *dev, const
struct panel_desc *desc)
panel_simple_parse_panel_timing_node(dev, panel, );
}
+ if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) {
+ /* Optional data-mapping property for overriding bus format */
+ err = panel_simple_override_nondefault_lvds_datamapping(dev,
panel);
+ if (err)
+ goto free_ddc;
+ }
+
connector_type = desc->connector_type;
/* Catch common mistakes for panels. */
switch (connector_type) {
--
2.39.2
[PATCH v5 2/3] dt-bindings: display: simple: support non-default data-mapping
Some Displays support more than just a single default LVDS data mapping,
which can be used to run displays on only 3 LVDS lanes in the jeida-18
data-mapping mode.
Add an optional data-mapping property to allow overriding the default
data mapping. As it does not generally apply to any display and bus, use
it selectively on the innolux,g101ice-l01, which supports changing the
data mapping via a strapping pin.
Reviewed-by: Conor Dooley
Signed-off-by: Johannes Zink
---
Changes:
v4 -> v5: none, re-added lost reviewed-by tag
v3 -> v4: none
v2 -> v3: - worked in Laurent's review findings (thanks for reviewing
my work): fix typos in commit message
v1 -> v2: - worked in Rob's review findings (thanks for reviewing my
work): use extracted common property instead of duplicating
the property
- refined commit message
- add an example dts for automated checking
---
.../bindings/display/panel/panel-simple.yaml | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 50143fe67471..81ac402ceed1 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -21,9 +21,9 @@ description: |
allOf:
- $ref: panel-common.yaml#
+ - $ref: ../lvds-data-mapping.yaml#
properties:
-
compatible:
enum:
# compatible must be listed in alphabetical order, ordered by compatible.
@@ -359,6 +359,17 @@ properties:
power-supply: true
no-hpd: true
hpd-gpios: true
+ data-mapping: true
+
+if:
+ not:
+properties:
+ compatible:
+contains:
+ const: innolux,g101ice-l01
+then:
+ properties:
+data-mapping: false
additionalProperties: false
@@ -378,3 +389,16 @@ examples:
};
};
};
+ - |
+panel_lvds: panel-lvds {
+ compatible = "innolux,g101ice-l01";
+ power-supply = <_lcd_reg>;
+
+ data-mapping = "jeida-24";
+
+ port {
+panel_in_lvds: endpoint {
+ remote-endpoint = <_out_lvds>;
+};
+ };
+};
--
2.39.2
[PATCH v5 1/3] dt-bindings: display: move LVDS data-mapping definition to separate file
As the LVDS data-mapping property is required in multiple bindings: move it to separate file and include instead of duplicating it. Reviewed-by: Conor Dooley Reviewed-by: Laurent Pinchart Signed-off-by: Johannes Zink --- Changes: v4 -> v5: none, but are-dded the reviewed-bys from v2, that were missing in v3 -> v4: none v2 -> v3: worked in Conor's and Laurent's review findings (thank you for reviewing my work): drop +| on description v1 -> v2: worked in Rob's review findings (thank you for reviewing my work): extract common properties to file and include it instead of duplicating it --- .../bindings/display/lvds-data-mapping.yaml| 84 ++ .../devicetree/bindings/display/lvds.yaml | 77 +++- 2 files changed, 93 insertions(+), 68 deletions(-) diff --git a/Documentation/devicetree/bindings/display/lvds-data-mapping.yaml b/Documentation/devicetree/bindings/display/lvds-data-mapping.yaml new file mode 100644 index ..d68982fe2e9b --- /dev/null +++ b/Documentation/devicetree/bindings/display/lvds-data-mapping.yaml @@ -0,0 +1,84 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/lvds-data-mapping.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LVDS Data Mapping + +maintainers: + - Laurent Pinchart + - Thierry Reding + +description: | + LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple + incompatible data link layers have been used over time to transmit image data + to LVDS devices. This bindings supports devices compatible with the following + specifications. + + [JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February + 1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA) + [LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National + Semiconductor + [VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video + Electronics Standards Association (VESA) + + Device compatible with those specifications have been marketed under the + FPD-Link and FlatLink brands. + +properties: + data-mapping: +enum: + - jeida-18 + - jeida-24 + - vesa-24 +description: | + The color signals mapping order. + + LVDS data mappings are defined as follows. + + - "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and +[VESA] specifications. Data are transferred as follows on 3 LVDS lanes. + + Slot 0 1 2 3 4 5 6 + _ + Clock \___/ + __ __ __ __ __ __ __ + DATA0 ><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__>< + DATA1 ><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__>< + DATA2 ><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__>< + + - "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI] +specifications. Data are transferred as follows on 4 LVDS lanes. + + Slot 0 1 2 3 4 5 6 + _ + Clock \___/ + __ __ __ __ __ __ __ + DATA0 ><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__>< + DATA1 ><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__>< + DATA2 ><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__>< + DATA3 ><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__>< + + - "vesa-24" - 24-bit data mapping compatible with the [VESA] specification. +Data are transferred as follows on 4 LVDS lanes. + + Slot 0 1 2 3 4 5 6 + _ + Clock \___/ + __ __ __ __ __ __ __ + DATA0 ><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__>< + DATA1 ><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__>< + DATA2 ><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__>< + DATA3 ><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__>< + + Control signals are mapped as follows. + + CTL0: HSync + CTL1: VSync + CTL2: Data Enable + CTL3: 0 + +additionalProperties: true + +... diff --git a/Documentation/devicetree/bindings/display/lvds.yaml b/Documentation/devicetree/bindings/display/lvds.yaml index 7cd2ce7e9c33..224db4932011 100644 --- a/Documentation/devicetree/bindings/display/lvds.yaml +++ b/Documentation/devicetree/bindings/display/lvds.yaml @@
[PATCH v5 0/3] Support non-default LVDS data mapping for simple panel
Some LVDS panels, such as the innolux,g101ice-l01 support multiple LVDS data mapping modes, which can be configured by strapping a dataformat pin on the display to a specific voltage. This can be particularly useful for using the jeida-18 format, which requires only 3 instead of 4 LVDS lanes. This series moves the data-mapping property for LVDS panels in a separate file and optionally adds it to simple-panel when matching to the innolux,g101ice-l01 compatible. This property allows to override the default data mapping set in the panel description in simple-panel. The last patch in this series actually adds the driver support for parsing the data format override device tree property and modifying the corresponding values for bit per color and media bus format in the panel descriptor. Best regards Johannes --- Changelog: v4 -> v5: - no changes, but added the reviewed-by-tags from v3 that were lost in v3/v4 - Link to v4: https://lore.kernel.org/r/20230523-simplepanel_support_nondefault_datamapping-v4-0-e6e7011f3...@pengutronix.de v3 -> v4: - driver: worked in Dan's Feedback: - return with proper error in case the call into panel_simple_override_nondefault_lvds_datamapping() failed - drop the unneeded and ambiguous ret local value - Link to v3: https://lore.kernel.org/r/20230523-simplepanel_support_nondefault_datamapping-v3-0-78ede374d...@pengutronix.de v2 -> v3: - dt bindings: Worked in Conor's and Laurent's Feedback (thanks for your review): Drop the chomping indicator - dt bindings: Worked in Laurent's Feedback: fix typos - driver: worked in Laurent's review findings: - extract function for fixing up the bus format - only call this function on LVDS panels - fix typo - Link to v2: https://lore.kernel.org/r/20230523-simplepanel_support_nondefault_datamapping-v2-0-87196f0d0...@pengutronix.de v1 -> v2: - dt bindings: Worked in Rob's review findings (thanks for your review), refactored to use common include instead of duplication - driver: added missing error unwinding goto, as found by Dan Carpenter's test robot: Reported-by: kernel test robot Reported-by: Dan Carpenter Link: https://lore.kernel.org/r/202304160359.4lhmfolu-...@intel.com/ To: David Airlie To: Daniel Vetter To: Rob Herring To: Krzysztof Kozlowski To: Conor Dooley To: Laurent Pinchart To: Thierry Reding To: Neil Armstrong To: Sam Ravnborg Cc: patchwork-...@pengutronix.de Cc: ker...@pengutronix.de Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Cc: devicet...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Johannes Zink --- --- Johannes Zink (3): dt-bindings: display: move LVDS data-mapping definition to separate file dt-bindings: display: simple: support non-default data-mapping drm/panel-simple: allow LVDS format override .../bindings/display/lvds-data-mapping.yaml| 84 ++ .../devicetree/bindings/display/lvds.yaml | 77 +++- .../bindings/display/panel/panel-simple.yaml | 26 ++- drivers/gpu/drm/panel/panel-simple.c | 53 ++ 4 files changed, 171 insertions(+), 69 deletions(-) --- base-commit: 79fb229b8810071648b65c37382aea7819a5f935 change-id: 20230523-simplepanel_support_nondefault_datamapping-13c3f2ea28f8 Best regards, -- Johannes Zink
Re: [PATCH 01/15] cdrom: Remove now superfluous sentinel element from ctl_table array
On Thu, Sep 28, 2023 at 03:36:55PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 28, 2023 at 03:21:26PM +0200, Joel Granados via B4 Relay wrote:
> > From: Joel Granados
> >
> > This commit comes at the tail end of a greater effort to remove the
> > empty elements at the end of the ctl_table arrays (sentinels) which
> > will reduce the overall build time size of the kernel and run time
> > memory bloat by ~64 bytes per sentinel (further information Link :
> > https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
> >
> > Remove sentinel element from cdrom_table
> >
> > Signed-off-by: Joel Granados
> > ---
> > drivers/cdrom/cdrom.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> > index cc2839805983..451907ade389 100644
> > --- a/drivers/cdrom/cdrom.c
> > +++ b/drivers/cdrom/cdrom.c
> > @@ -3654,8 +3654,7 @@ static struct ctl_table cdrom_table[] = {
> > .maxlen = sizeof(int),
> > .mode = 0644,
> > .proc_handler = cdrom_sysctl_handler
> > - },
> > - { }
> > + }
>
> You should have the final entry as "}," so as to make any future
> additions to the list to only contain that entry, that's long been the
> kernel style for lists like this.
Will send a V2 with this included. Thx.
>
> So your patches will just remove one line, not 2 and add 1, making it a
> smaller diff.
indeed.
>
> thanks,
>
> greg k-h
--
Joel Granados
signature.asc
Description: PGP signature
Re: [PATCH 14/15] hyper-v/azure: Remove now superfluous sentinel element from ctl_table array
On Thu, Sep 28, 2023 at 03:26:16PM +, Wei Liu wrote:
> Please change the prefix to "Drivers: hv:" in the subject line in the
> two patches.
>
> On Thu, Sep 28, 2023 at 03:21:39PM +0200, Joel Granados via B4 Relay wrote:
> > From: Joel Granados
> >
> > This commit comes at the tail end of a greater effort to remove the
> > empty elements at the end of the ctl_table arrays (sentinels) which
> > will reduce the overall build time size of the kernel and run time
> > memory bloat by ~64 bytes per sentinel (further information Link :
> > https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
> >
> > Remove sentinel from hv_ctl_table
> >
> > Signed-off-by: Joel Granados
> > ---
> > drivers/hv/hv_common.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> > index ccad7bca3fd3..bc7d678030aa 100644
> > --- a/drivers/hv/hv_common.c
> > +++ b/drivers/hv/hv_common.c
> > @@ -147,8 +147,7 @@ static struct ctl_table hv_ctl_table[] = {
> > .proc_handler = proc_dointvec_minmax,
> > .extra1 = SYSCTL_ZERO,
> > .extra2 = SYSCTL_ONE
> > - },
> > - {}
> > + }
>
> Please keep the comma at the end.
My V2 will have this.
>
> > };
> >
> > static int hv_die_panic_notify_crash(struct notifier_block *self,
> >
> > --
> > 2.30.2
> >
--
Joel Granados
signature.asc
Description: PGP signature
Re: [PATCH 11/15] sgi-xp: Remove the now superfluous sentinel element from ctl_table array
On Thu, Sep 28, 2023 at 12:51:15PM -0500, Steve Wahl wrote:
> On Thu, Sep 28, 2023 at 03:21:36PM +0200, Joel Granados via B4 Relay wrote:
> > From: Joel Granados
> >
> > This commit comes at the tail end of a greater effort to remove the
> > empty elements at the end of the ctl_table arrays (sentinels) which
> > will reduce the overall build time size of the kernel and run time
> > memory bloat by ~64 bytes per sentinel (further information Link :
> > https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
> >
> > Remove sentinel from xpc_sys_xpc_hb and xpc_sys_xpc
> >
> > Signed-off-by: Joel Granados
> > ---
> > drivers/misc/sgi-xp/xpc_main.c | 6 ++
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/misc/sgi-xp/xpc_main.c b/drivers/misc/sgi-xp/xpc_main.c
> > index 6da509d692bb..c898092ff3ac 100644
> > --- a/drivers/misc/sgi-xp/xpc_main.c
> > +++ b/drivers/misc/sgi-xp/xpc_main.c
> > @@ -109,8 +109,7 @@ static struct ctl_table xpc_sys_xpc_hb[] = {
> > .mode = 0644,
> > .proc_handler = proc_dointvec_minmax,
> > .extra1 = _hb_check_min_interval,
> > -.extra2 = _hb_check_max_interval},
> > - {}
> > +.extra2 = _hb_check_max_interval}
> > };
> > static struct ctl_table xpc_sys_xpc[] = {
> > {
> > @@ -120,8 +119,7 @@ static struct ctl_table xpc_sys_xpc[] = {
> > .mode = 0644,
> > .proc_handler = proc_dointvec_minmax,
> > .extra1 = _disengage_min_timelimit,
> > -.extra2 = _disengage_max_timelimit},
> > - {}
> > +.extra2 = _disengage_max_timelimit}
> > };
> >
> > static struct ctl_table_header *xpc_sysctl;
> >
> > --
> > 2.30.2
> >
>
> I assume you'll match the rest of the changes with regards to the
> trailing comma.
If you are refering to Greg's comments. yes. I'll send a V2 with the
trailing comma.
Best
>
> Reviewed-by: Steve Wahl
>
> --
> Steve Wahl, Hewlett Packard Enterprise
--
Joel Granados
signature.asc
Description: PGP signature
Re: [PATCH v2 0/5] drm: Reuse temporary memory for format conversion
On Wed, Sep 20, 2023 at 04:24:26PM +0200, Thomas Zimmermann wrote: > DRM's format-conversion helpers require temporary memory. Pass the > buffer from the caller and keep it allocated over several calls. Allow > the caller to preallocate the buffer memory. > > The motivation for this patchset is the recent work on a DRM panic > handler. The panic handler requires format conversion to display an > error to the screen. But allocating memory during kernel panics is > fragile. The changes in this patchset enable the DRM panic handler to > preallocate buffer storage before the panic occurs. > > As an additonal benefit, drivers can now keep the temporary storage > across multiple display updates. Avoiding memory allocation reduces > the CPU overhead of the format helpers. This argument is getting a bit tiring. The main reason is that it isn't one, and: - we allocate something in the 10-20 range objects at a given commit, so another small one is not insignificant. - If it was, it would be trivial to produce a benchmark, but no-one ever actually showed a workload and numbers where there's actually any difference. - Also, the CPU overhead is indeed (even if marginally) decreased, but the memory overhead is increased. I don't know whether that's a good trade-off or not, see the point above. It really sounds like an empty statement to me: "But just think of the CPU!". That being said, I also have more fundamental doubts about this series. The first one is that storing the buffer pointer in the device instead of the state makes it harder to reason about. When you have a state, the framework provides the guarantee at commit time that there's only going to be one at a given time. And since the buffer is stored in that state object, you can't access it by mistake. The buffer size also depends on the state, so that all makes sense from a logical PoV. If we store the buffer into the device, then suddenly you have to think about whether there's multiple CRTCs or not (since commits aren't serialised if they affect different CRTCs), whether the buffer size you allocated is large enough now for your current resolution and format, etc. It adds a decent chunk of complexity on something that was quite complex already. I understand that the motivation is for drm_panic to have a buffer ready when it has to kick in. But why don't we just allocate (and check) a spare commit at probe time so we just have to commit it when we panic. That would fall nicely into the rest of the atomic modesetting code, and since we pretty much require not to allocate anything during atomic_commit, we have that constraints already figured out. Maxime signature.asc Description: PGP signature
[PATCH v3 03/10] drm/amdgpu: Use RMW accessors for changing LNKCTL2
Don't assume that only the driver would be accessing LNKCTL2. In the case of upstream (parent), the driver does not even own the device it's changing the registers for. Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value. This change is also useful as a cleanup. Suggested-by: Lukas Wunner Signed-off-by: Ilpo Järvinen --- drivers/gpu/drm/amd/amdgpu/cik.c | 41 drivers/gpu/drm/amd/amdgpu/si.c | 41 2 files changed, 30 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index e63abdf52b6c..7bcd41996927 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1638,28 +1638,18 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) PCI_EXP_LNKCTL_HAWD); /* linkctl2 */ - pcie_capability_read_word(root, PCI_EXP_LNKCTL2, - ); - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN); - tmp16 |= (bridge_cfg2 & - (PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN)); - pcie_capability_write_word(root, - PCI_EXP_LNKCTL2, - tmp16); - - pcie_capability_read_word(adev->pdev, - PCI_EXP_LNKCTL2, - ); - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN); - tmp16 |= (gpu_cfg2 & - (PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN)); - pcie_capability_write_word(adev->pdev, - PCI_EXP_LNKCTL2, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN, + bridge_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); + pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN, + gpu_cfg2 & + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); tmp = RREG32_PCIE(ixPCIE_LC_CNTL4); tmp &= ~PCIE_LC_CNTL4__LC_SET_QUIESCE_MASK; @@ -1674,16 +1664,15 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) speed_cntl &= ~PCIE_LC_SPEED_CNTL__LC_FORCE_DIS_SW_SPEED_CHANGE_MASK; WREG32_PCIE(ixPCIE_LC_SPEED_CNTL, speed_cntl); - pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL2, ); - tmp16 &= ~PCI_EXP_LNKCTL2_TLS; - + tmp16 = 0; if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */ else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2) tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */ else tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */ - pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL2, tmp16); + pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_TLS, tmp16); speed_cntl = RREG32_PCIE(ixPCIE_LC_SPEED_CNTL); speed_cntl |= PCIE_LC_SPEED_CNTL__LC_INITIATE_LINK_SPEED_CHANGE_MASK; diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c index 4b81f29e5fd5..8ea60fdd1b1d 100644 --- a/drivers/gpu/drm/amd/amdgpu/si.c +++ b/drivers/gpu/drm/amd/amdgpu/si.c @@ -2331,28 +2331,18 @@
[PATCH v3 02/10] drm/radeon: Use RMW accessors for changing LNKCTL2
Don't assume that only the driver would be accessing LNKCTL2. In the case of upstream (parent), the driver does not even own the device it's changing the registers for. Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value. This change is also useful as a cleanup. Suggested-by: Lukas Wunner Signed-off-by: Ilpo Järvinen --- drivers/gpu/drm/radeon/cik.c | 40 ++-- drivers/gpu/drm/radeon/si.c | 40 ++-- 2 files changed, 30 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 10be30366c2b..b5e96a8fc2c1 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9592,28 +9592,18 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) PCI_EXP_LNKCTL_HAWD); /* linkctl2 */ - pcie_capability_read_word(root, PCI_EXP_LNKCTL2, - ); - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN); - tmp16 |= (bridge_cfg2 & - (PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN)); - pcie_capability_write_word(root, - PCI_EXP_LNKCTL2, - tmp16); - - pcie_capability_read_word(rdev->pdev, - PCI_EXP_LNKCTL2, - ); - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN); - tmp16 |= (gpu_cfg2 & - (PCI_EXP_LNKCTL2_ENTER_COMP | - PCI_EXP_LNKCTL2_TX_MARGIN)); - pcie_capability_write_word(rdev->pdev, - PCI_EXP_LNKCTL2, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN, + bridge_cfg2 | + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN, + gpu_cfg2 | + (PCI_EXP_LNKCTL2_ENTER_COMP | + PCI_EXP_LNKCTL2_TX_MARGIN)); tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4); tmp &= ~LC_SET_QUIESCE; @@ -9627,15 +9617,15 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE; WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl); - pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL2, ); - tmp16 &= ~PCI_EXP_LNKCTL2_TLS; + tmp16 = 0; if (speed_cap == PCIE_SPEED_8_0GT) tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */ else if (speed_cap == PCIE_SPEED_5_0GT) tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */ else tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */ - pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL2, tmp16); + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_TLS, tmp16); speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL); speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE; diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index a91012447b56..32871ca09a0f 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7189,28 +7189,18 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) PCI_EXP_LNKCTL_HAWD);
Re: [PATCH 2/2] drm/bridge: lt9611uxc: use drm_bridge_get_edid() instead of using ->get_edid directly
On Thu, 28 Sep 2023, Laurent Pinchart wrote: > On Wed, Sep 27, 2023 at 05:09:23PM +0300, Jani Nikula wrote: >> On Wed, 27 Sep 2023, Laurent Pinchart wrote: >> > On Thu, Sep 14, 2023 at 04:14:50PM +0300, Jani Nikula wrote: >> >> Make drm_bridge_get_edid() the one place to call the hook. >> >> >> >> Cc: Andrzej Hajda >> >> Cc: Neil Armstrong >> >> Cc: Robert Foss >> >> Cc: Laurent Pinchart >> >> Cc: Jonas Karlman >> >> Cc: Jernej Skrabec >> >> Signed-off-by: Jani Nikula >> >> >> >> --- >> >> >> >> UNTESTED >> > >> > I can't test this either, but it looks fine. >> >> Thanks. Are you okay with merging the two with review only? > > The changes are trivial, if we can't get anyone to test them, then I'm > OK merging them. Thanks, pushed to drm-misc-next. BR, Jani. > >> > Reviewed-by: Laurent Pinchart >> > >> >> --- >> >> drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c >> >> b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c >> >> index 22c84d29c2bc..7835738a532e 100644 >> >> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c >> >> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c >> >> @@ -296,7 +296,7 @@ static int lt9611uxc_connector_get_modes(struct >> >> drm_connector *connector) >> >> unsigned int count; >> >> struct edid *edid; >> >> >> >> - edid = lt9611uxc->bridge.funcs->get_edid(>bridge, connector); >> >> + edid = drm_bridge_get_edid(>bridge, connector); >> >> drm_connector_update_edid_property(connector, edid); >> >> count = drm_add_edid_modes(connector, edid); >> >> kfree(edid); -- Jani Nikula, Intel
Re: [PATCH 00/19] drm/i915: prepare for xe driver display integration
On Thu, 14 Sep 2023, Rodrigo Vivi wrote: > On Tue, Sep 12, 2023 at 02:06:27PM +0300, Jani Nikula wrote: >> The upcoming drm/xe driver [1][2] will reuse the drm/i915 display code, >> initially by compiling the relevant compilation units separately as part >> of the xe driver. This series prepares for that in i915 side. >> >> The first patch defines I915 during the i915 driver build, to allow >> conditional compilation based on the driver the code is being built for. >> >> The rest of the patches add stubs for functions in files that aren't >> used in xe. The idea is that this is the least intrusive way of skipping >> that code in xe, and is quite similar to the common kconfig stubs. >> >> While this is arguably unused code for the time being, or only used in >> an out-of-tree driver yet to be upstreamed, the upstreaming has to start >> somewhere. > > I see other benefits on adding this right now through drm-intel-next: > > 1. Separate the good patches from the other patches that are in >drm-xe-next, that would require more work. > 2. Minimize the non-xe patches in the xe pull-request. Cleaner and with >reduced risk of conflicts. > >> >> >> BR, >> Jani. >> >> >> [1] https://gitlab.freedesktop.org/drm/xe/kernel/-/tree/drm-xe-next >> [2] https://patchwork.freedesktop.org/series/112188/ >> >> Cc: David Airlie >> Cc: Daniel Vetter >> Cc: Joonas Lahtinen >> Cc: Rodrigo Vivi >> Cc: Tvrtko Ursulin >> Cc: Lucas De Marchi > > Yeap, let's for sure get input from other maintainers, but meanwhile I'd > like to state that I have once more reviewed these patches and that > you can put my rv-b tag in all of them when we get the proper acks. Thanks. Pushed to drm-intel-next with Dave's IRC ack. BR, Jani. > >> >> Jani Nikula (19): >> drm/i915: define I915 during i915 driver build >> drm/i915/display: add I915 conditional build to intel_lvds.h >> drm/i915/display: add I915 conditional build to hsw_ips.h >> drm/i915/display: add I915 conditional build to i9xx_plane.h >> drm/i915/display: add I915 conditional build to intel_lpe_audio.h >> drm/i915/display: add I915 conditional build to intel_pch_refclk.h >> drm/i915/display: add I915 conditional build to intel_pch_display.h >> drm/i915/display: add I915 conditional build to intel_sprite.h >> drm/i915/display: add I915 conditional build to intel_overlay.h >> drm/i915/display: add I915 conditional build to g4x_dp.h >> drm/i915/display: add I915 conditional build to intel_dpio_phy.h >> drm/i915/display: add I915 conditional build to intel_crt.h >> drm/i915/display: add I915 conditional build to vlv_dsi.h >> drm/i915/display: add I915 conditional build to i9xx_wm.h >> drm/i915/display: add I915 conditional build to g4x_hdmi.h >> drm/i915/display: add I915 conditional build to intel_dvo.h >> drm/i915/display: add I915 conditional build to intel_sdvo.h >> drm/i915/display: add I915 conditional build to intel_tv.h >> drm/i915/display: add I915 conditional build to vlv_dsi_pll.h >> >> drivers/gpu/drm/i915/Makefile | 4 + >> drivers/gpu/drm/i915/display/g4x_dp.h | 26 + >> drivers/gpu/drm/i915/display/g4x_hdmi.h | 12 +++ >> drivers/gpu/drm/i915/display/hsw_ips.h| 35 +++ >> drivers/gpu/drm/i915/display/i9xx_plane.h | 23 + >> drivers/gpu/drm/i915/display/i9xx_wm.h| 17 >> drivers/gpu/drm/i915/display/intel_crt.h | 14 +++ >> drivers/gpu/drm/i915/display/intel_dpio_phy.h | 96 +++ >> drivers/gpu/drm/i915/display/intel_dvo.h | 6 ++ >> .../gpu/drm/i915/display/intel_lpe_audio.h| 18 >> drivers/gpu/drm/i915/display/intel_lvds.h | 19 >> drivers/gpu/drm/i915/display/intel_overlay.h | 35 +++ >> .../gpu/drm/i915/display/intel_pch_display.h | 53 ++ >> .../gpu/drm/i915/display/intel_pch_refclk.h | 23 + >> drivers/gpu/drm/i915/display/intel_sdvo.h | 13 +++ >> drivers/gpu/drm/i915/display/intel_sprite.h | 8 ++ >> drivers/gpu/drm/i915/display/intel_tv.h | 6 ++ >> drivers/gpu/drm/i915/display/vlv_dsi.h| 13 +++ >> drivers/gpu/drm/i915/display/vlv_dsi_pll.h| 9 ++ >> 19 files changed, 430 insertions(+) >> >> -- >> 2.39.2 >> -- Jani Nikula, Intel
[PULL] drm-intel-next
Hi Dave & Daniel - drm-intel-next-2023-09-29: drm/i915 feature pull for v6.7: Features and functionality: - Early Xe2 LPD / Lunarlake (LNL) display enabling (Lucas, Matt, Gustavo, Stanislav, Luca, Clint, Juha-Pekka, Balasubramani, Ravi) - Plenty of various DSC improvements and fixes (Ankit) - Add DSC PPS state readout and verification (Suraj) - Improve fastsets for VRR, LRR and M/N updates (Ville) - Use connector->ddc to create (non-DP MST) connector sysfs ddc symlinks (Ville) - Various DSB improvements, load LUTs using DSB (Ville) - Improve shared link bandwidth management, starting with FDI (Imre) - Optimize get param ioctl for PXP status (Alan) - Remove DG2 pre-production hardware workarounds (Matt) - Add more RPL P/U PCI IDs (Dnyaneshwar) - Add new DG2-G12 stepping (Swati) - Add PSR sink error status to debugfs (Jouni) - Add DP enhanced framing to crtc state checker (Ville) Refactoring and cleanups: - Simplify TileY/Tile4 tiling selftest enumeration (Matt) - Remove some unused power domain code (Gustavo) - Check stepping of display IP version rather than MTL platform (Matt) - DP audio compute config cleanups (Vinod) - SDVO cleanups and refactoring, more robust failure handling (Ville) - Color register definition and readout cleanups (Jani) - Reduce header interdependencies for frontbuffer tracking (Jani) - Continue replacing struct edid with struct drm_edid (Jani) - Use source physical address instead of EDID for CEC (Jani) - Clean up Type-C port lane count functions (Luca) - Clean up DSC PPS register definitions and readout (Jani) - Stop using GEM_BUG_ON()/GEM_WARN_ON() in display code (Jani) - Move more of the display probe to display code (Jani) - Remove redundant runtime suspended state flag (Jouni) - Move display info printing to display code (Balasubramani) - Frontbuffer tracking improvements (Jouni) - Add trailing newlines to debug logging (Jim Cromie) - Separate display workarounds from clock gating init (Matt) - Reduce dmesg log spamming for combo PHY, PLL state, FEC, DP MST (Ville, Imre) Fixes: - Fix hotplug poll detect loops via suspend/resume (Imre) - Fix hotplug detect for forced connectors (Imre) - Fix DSC first_line_bpg_offset calculation (Suraj) - Fix debug prints for SDP CRC16 (Arun) - Fix PXP runtime resume (Alan) - Fix cx0 PHY lane handling (Gustavo) - Fix frontbuffer tracking locking in debugfs (Juha-Pekka) - Fix SDVO detect on some models (Ville) - Fix SDP split configuration for DP MST (Vinod) - Fix AUX usage and reads for HDCP on DP MST (Suraj) - Fix PSR workaround (Jouni) - Fix redundant AUX power get/put in DP force (Imre) - Fix ICL DSI TCLK POST by letting hardware handle it (William) - Fix IRQ reset for XE LP+ (Gustavo) - Fix h/vsync_end instead of h/vtotal in VBT (Ville) - Fix C20 PHY msgbus timeout issues (Gustavo) - Fix pre-TGL FEC pipe A vs. DDI A mixup (Ville) - Fix FEC state readout for DP MST (Ville) DRM subsystem core changes: - Assume sink supports 8 bpc when DSC is supported (Ankit) - Add drm_edid_is_digital() helper (Jani) - Parse source physical address from EDID (Jani) - Add function to attach CEC without EDID (Jani) - Reorder connector sysfs/debugfs remove (Ville) - Register connector sysfs ddc symlink later (Ville) Media subsystem changes: - Add comments about CEC source physical address usage (Jani) Merges: - Backmerge drm-next to get v6.6-rc1 (Jani) BR, Jani. The following changes since commit 0bb80ecc33a8fb5a682236443c1e740d5c917d1d: Linux 6.6-rc1 (2023-09-10 16:28:41 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-2023-09-29 for you to fetch changes up to 3570bd989acc66add5726785058cceffa06b1f54: drm/i915: Update DRIVER_DATE to 20230929 (2023-09-29 12:43:23 +0300) drm/i915 feature pull for v6.7: Features and functionality: - Early Xe2 LPD / Lunarlake (LNL) display enabling (Lucas, Matt, Gustavo, Stanislav, Luca, Clint, Juha-Pekka, Balasubramani, Ravi) - Plenty of various DSC improvements and fixes (Ankit) - Add DSC PPS state readout and verification (Suraj) - Improve fastsets for VRR, LRR and M/N updates (Ville) - Use connector->ddc to create (non-DP MST) connector sysfs ddc symlinks (Ville) - Various DSB improvements, load LUTs using DSB (Ville) - Improve shared link bandwidth management, starting with FDI (Imre) - Optimize get param ioctl for PXP status (Alan) - Remove DG2 pre-production hardware workarounds (Matt) - Add more RPL P/U PCI IDs (Dnyaneshwar) - Add new DG2-G12 stepping (Swati) - Add PSR sink error status to debugfs (Jouni) - Add DP enhanced framing to crtc state checker (Ville) Refactoring and cleanups: - Simplify TileY/Tile4 tiling selftest enumeration (Matt) - Remove some unused power domain code (Gustavo) - Check stepping of display IP version rather than MTL platform (Matt) - DP audio compute config cleanups (Vinod) - SDVO cleanups and refactoring, more robust failure handl
[PATCH] dma-buf: add dma_fence_timestamp helper
When a fence signals there is a very small race window where the timestamp
isn't updated yet. sync_file solves this by busy waiting for the
timestamp to appear, but on other ocassions didn't handled this
correctly.
Provide a dma_fence_timestamp() helper function for this and use it in
all appropriate cases.
Another alternative would be to grab the spinlock when that happens.
v2 by teddy: add a wait parameter to wait for the timestamp to show up, in case
the accurate timestamp is needed and/or the timestamp is not based on
ktime (e.g. hw timestamp)
v3 chk: drop the parameter again for unified handling
Signed-off-by: Yunxiang Li
Signed-off-by: Christian König
Fixes: 1774baa64f93 ("drm/scheduler: Change scheduled fence track v2")
---
drivers/dma-buf/dma-fence-unwrap.c | 13 -
drivers/dma-buf/sync_file.c| 9 +++--
drivers/gpu/drm/scheduler/sched_main.c | 2 +-
include/linux/dma-fence.h | 19 +++
4 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c
b/drivers/dma-buf/dma-fence-unwrap.c
index c625bb2b5d56..628af51c81af 100644
--- a/drivers/dma-buf/dma-fence-unwrap.c
+++ b/drivers/dma-buf/dma-fence-unwrap.c
@@ -76,16 +76,11 @@ struct dma_fence *__dma_fence_unwrap_merge(unsigned int
num_fences,
dma_fence_unwrap_for_each(tmp, [i], fences[i]) {
if (!dma_fence_is_signaled(tmp)) {
++count;
- } else if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT,
- >flags)) {
- if (ktime_after(tmp->timestamp, timestamp))
- timestamp = tmp->timestamp;
} else {
- /*
-* Use the current time if the fence is
-* currently signaling.
-*/
- timestamp = ktime_get();
+ ktime_t t = dma_fence_timestamp(tmp);
+
+ if (ktime_after(t, timestamp))
+ timestamp = t;
}
}
}
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index af57799c86ce..2e9a316c596a 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -268,13 +268,10 @@ static int sync_fill_fence_info(struct dma_fence *fence,
sizeof(info->driver_name));
info->status = dma_fence_get_status(fence);
- while (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags) &&
- !test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags))
- cpu_relax();
info->timestamp_ns =
- test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags) ?
- ktime_to_ns(fence->timestamp) :
- ktime_set(0, 0);
+ dma_fence_is_signaled(fence) ?
+ ktime_to_ns(dma_fence_timestamp(fence)) :
+ ktime_set(0, 0);
return info->status;
}
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 506371c42745..5a3a622fc672 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -929,7 +929,7 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
if (next) {
next->s_fence->scheduled.timestamp =
- job->s_fence->finished.timestamp;
+ dma_fence_timestamp(>s_fence->finished);
/* start TO timer for next job */
drm_sched_start_timeout(sched);
}
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 0d678e9a7b24..ebe78bd3d121 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -568,6 +568,25 @@ static inline void dma_fence_set_error(struct dma_fence
*fence,
fence->error = error;
}
+/**
+ * dma_fence_timestamp - helper to get the completion timestamp of a fence
+ * @fence: fence to get the timestamp from.
+ *
+ * After a fence is signaled the timestamp is updated with the signaling time,
+ * but setting the timestamp can race with tasks waiting for the signaling.
This
+ * helper busy waits for the correct timestamp to appear.
+ */
+static inline ktime_t dma_fence_timestamp(struct dma_fence *fence)
+{
+ if (WARN_ON(!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)))
+ return ktime_get();
+
+ while (!test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags))
+ cpu_relax();
+
+ return fence->timestamp;
+}
+
signed long dma_fence_wait_timeout(struct dma_fence *,
bool intr, signed long timeout);
signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
--
Re: [PATCH v8] drm/doc: Document DRM device reset expectations
Am 29.09.23 um 11:25 schrieb André Almeida: Create a section that specifies how to deal with DRM device resets for kernel and userspace drivers. Acked-by: Pekka Paalanen Acked-by: Sebastian Wick Signed-off-by: André Almeida Reviewed-by: Christian König I think that is now ready to be pushed. Do you have commit rights to drm-misc-next? If not please ping me. Regards, Christian. --- v8 changes: - Add acked-by tags v7: https://lore.kernel.org/dri-devel/20230818200642.276735-1-andrealm...@igalia.com/ v7 changes: - s/application/graphical API contex/ in the robustness part (Michel) - Grammar fixes (Randy) v6: https://lore.kernel.org/lkml/20230815185710.159779-1-andrealm...@igalia.com/ v6 changes: - Due to substantial changes in the content, dropped Pekka's Acked-by - Grammar fixes (Randy) - Add paragraph about disabling device resets - Add note about integrating reset tracking in drm/sched - Add note that KMD should return failure for contexts affected by resets and UMD should check for this - Add note about lack of consensus around what to do about non-robust apps v5: https://lore.kernel.org/dri-devel/20230627132323.115440-1-andrealm...@igalia.com/ --- Documentation/gpu/drm-uapi.rst | 77 ++ 1 file changed, 77 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index eef5fd19bc92..632989df3727 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -285,6 +285,83 @@ for GPU1 and GPU2 from different vendors, and a third handler for mmapped regular files. Threads cause additional pain with signal handling as well. +Device reset + + +The GPU stack is really complex and is prone to errors, from hardware bugs, +faulty applications and everything in between the many layers. Some errors +require resetting the device in order to make the device usable again. This +section describes the expectations for DRM and usermode drivers when a +device resets and how to propagate the reset status. + +Device resets can not be disabled without tainting the kernel, which can lead to +hanging the entire kernel through shrinkers/mmu_notifiers. Userspace role in +device resets is to propagate the message to the application and apply any +special policy for blocking guilty applications, if any. Corollary is that +debugging a hung GPU context require hardware support to be able to preempt such +a GPU context while it's stopped. + +Kernel Mode Driver +-- + +The KMD is responsible for checking if the device needs a reset, and to perform +it as needed. Usually a hang is detected when a job gets stuck executing. KMD +should keep track of resets, because userspace can query any time about the +reset status for a specific context. This is needed to propagate to the rest of +the stack that a reset has happened. Currently, this is implemented by each +driver separately, with no common DRM interface. Ideally this should be properly +integrated at DRM scheduler to provide a common ground for all drivers. After a +reset, KMD should reject new command submissions for affected contexts. + +User Mode Driver + + +After command submission, UMD should check if the submission was accepted or +rejected. After a reset, KMD should reject submissions, and UMD can issue an +ioctl to the KMD to check the reset status, and this can be checked more often +if the UMD requires it. After detecting a reset, UMD will then proceed to report +it to the application using the appropriate API error code, as explained in the +section below about robustness. + +Robustness +-- + +The only way to try to keep a graphical API context working after a reset is if +it complies with the robustness aspects of the graphical API that it is using. + +Graphical APIs provide ways to applications to deal with device resets. However, +there is no guarantee that the app will use such features correctly, and a +userspace that doesn't support robust interfaces (like a non-robust +OpenGL context or API without any robustness support like libva) leave the +robustness handling entirely to the userspace driver. There is no strong +community consensus on what the userspace driver should do in that case, +since all reasonable approaches have some clear downsides. + +OpenGL +~~ + +Apps using OpenGL should use the available robust interfaces, like the +extension ``GL_ARB_robustness`` (or ``GL_EXT_robustness`` for OpenGL ES). This +interface tells if a reset has happened, and if so, all the context state is +considered lost and the app proceeds by creating new ones. There's no consensus +on what to do to if robustness is not in use. + +Vulkan +~~ + +Apps using Vulkan should check for ``VK_ERROR_DEVICE_LOST`` for submissions. +This error code means, among other things, that a device reset has happened and +it needs to recreate the contexts to keep going. + +Reporting causes of
Re: [PATCH v3 2/3] drm/panic: Add a drm panic handler
Hi Jocelyn,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 2dde18cd1d8fac735875f2e4987f11817cc0bc2c]
url:
https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Export-drm_fb_xrgb_to_rgb565_line/20230928-013030
base: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
patch link:
https://lore.kernel.org/r/20230927172849.193996-3-jfalempe%40redhat.com
patch subject: [PATCH v3 2/3] drm/panic: Add a drm panic handler
config: arc-randconfig-002-20230929
(https://download.01.org/0day-ci/archive/20230929/202309291753.8xaivqn0-...@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20230929/202309291753.8xaivqn0-...@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
| Closes:
https://lore.kernel.org/oe-kbuild-all/202309291753.8xaivqn0-...@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_panic.c:363: warning: expecting prototype for
>> drm_log_exit(). Prototype was for drm_panic_exit() instead
vim +363 drivers/gpu/drm/drm_panic.c
358
359 /**
360 * drm_log_exit() - Shutdown drm-panic subsystem
361 */
362 void drm_panic_exit(void)
> 363 {
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[PATCH v8] drm/doc: Document DRM device reset expectations
Create a section that specifies how to deal with DRM device resets for kernel and userspace drivers. Acked-by: Pekka Paalanen Acked-by: Sebastian Wick Signed-off-by: André Almeida --- v8 changes: - Add acked-by tags v7: https://lore.kernel.org/dri-devel/20230818200642.276735-1-andrealm...@igalia.com/ v7 changes: - s/application/graphical API contex/ in the robustness part (Michel) - Grammar fixes (Randy) v6: https://lore.kernel.org/lkml/20230815185710.159779-1-andrealm...@igalia.com/ v6 changes: - Due to substantial changes in the content, dropped Pekka's Acked-by - Grammar fixes (Randy) - Add paragraph about disabling device resets - Add note about integrating reset tracking in drm/sched - Add note that KMD should return failure for contexts affected by resets and UMD should check for this - Add note about lack of consensus around what to do about non-robust apps v5: https://lore.kernel.org/dri-devel/20230627132323.115440-1-andrealm...@igalia.com/ --- Documentation/gpu/drm-uapi.rst | 77 ++ 1 file changed, 77 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index eef5fd19bc92..632989df3727 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -285,6 +285,83 @@ for GPU1 and GPU2 from different vendors, and a third handler for mmapped regular files. Threads cause additional pain with signal handling as well. +Device reset + + +The GPU stack is really complex and is prone to errors, from hardware bugs, +faulty applications and everything in between the many layers. Some errors +require resetting the device in order to make the device usable again. This +section describes the expectations for DRM and usermode drivers when a +device resets and how to propagate the reset status. + +Device resets can not be disabled without tainting the kernel, which can lead to +hanging the entire kernel through shrinkers/mmu_notifiers. Userspace role in +device resets is to propagate the message to the application and apply any +special policy for blocking guilty applications, if any. Corollary is that +debugging a hung GPU context require hardware support to be able to preempt such +a GPU context while it's stopped. + +Kernel Mode Driver +-- + +The KMD is responsible for checking if the device needs a reset, and to perform +it as needed. Usually a hang is detected when a job gets stuck executing. KMD +should keep track of resets, because userspace can query any time about the +reset status for a specific context. This is needed to propagate to the rest of +the stack that a reset has happened. Currently, this is implemented by each +driver separately, with no common DRM interface. Ideally this should be properly +integrated at DRM scheduler to provide a common ground for all drivers. After a +reset, KMD should reject new command submissions for affected contexts. + +User Mode Driver + + +After command submission, UMD should check if the submission was accepted or +rejected. After a reset, KMD should reject submissions, and UMD can issue an +ioctl to the KMD to check the reset status, and this can be checked more often +if the UMD requires it. After detecting a reset, UMD will then proceed to report +it to the application using the appropriate API error code, as explained in the +section below about robustness. + +Robustness +-- + +The only way to try to keep a graphical API context working after a reset is if +it complies with the robustness aspects of the graphical API that it is using. + +Graphical APIs provide ways to applications to deal with device resets. However, +there is no guarantee that the app will use such features correctly, and a +userspace that doesn't support robust interfaces (like a non-robust +OpenGL context or API without any robustness support like libva) leave the +robustness handling entirely to the userspace driver. There is no strong +community consensus on what the userspace driver should do in that case, +since all reasonable approaches have some clear downsides. + +OpenGL +~~ + +Apps using OpenGL should use the available robust interfaces, like the +extension ``GL_ARB_robustness`` (or ``GL_EXT_robustness`` for OpenGL ES). This +interface tells if a reset has happened, and if so, all the context state is +considered lost and the app proceeds by creating new ones. There's no consensus +on what to do to if robustness is not in use. + +Vulkan +~~ + +Apps using Vulkan should check for ``VK_ERROR_DEVICE_LOST`` for submissions. +This error code means, among other things, that a device reset has happened and +it needs to recreate the contexts to keep going. + +Reporting causes of resets +-- + +Apart from propagating the reset through the stack so apps can recover, it's +really useful for driver developers to learn more about what caused the reset in +the first place. DRM devices
Re: [PATCH v2 5/5] drm/ssd130x: Store xfrm buffer in device instance
Thomas Zimmermann writes: > Store and instance of struct drm_xfrm_buf in struct ssd130x_device and > keep the allocated memory allocated across display updates. Avoid > possibly reallocating temporary memory on each display update. Instead > preallocate temporary memory during initialization. Releasing the DRM > device also releases the xfrm buffer. > > v2: > * reserve storage during probe > > Signed-off-by: Thomas Zimmermann > --- [...] > @@ -1084,6 +1081,8 @@ struct ssd130x_device *ssd130x_probe(struct device > *dev, struct regmap *regmap) > struct ssd130x_device *ssd130x; > struct backlight_device *bl; > struct drm_device *drm; > + const struct drm_format_info *fi; > + void *buf; > int ret; > > ssd130x = devm_drm_dev_alloc(dev, _drm_driver, > @@ -1117,6 +1116,18 @@ struct ssd130x_device *ssd130x_probe(struct device > *dev, struct regmap *regmap) > bl->props.max_brightness = MAX_CONTRAST; > ssd130x->bl_dev = bl; > > + ret = drmm_xfrm_buf_init(drm, >xfrm); > + if (ret) > + return ERR_PTR(ret); > + fi = drm_format_info(DRM_FORMAT_R1); > + if (!fi) > + return ERR_PTR(-EINVAL); > + buf = drm_xfrm_buf_reserve(>xfrm, > +drm_format_info_min_pitch(fi, 0, > ssd130x->width), > +GFP_KERNEL); > + if (!buf) > + return ERR_PTR(-ENOMEM); > + I think this is OK but then I wonder if we should not just allocate all the buffers in the probe function. Right now, what the driver does is to have two structures to keep the driver-private atomic state: 1) struct ssd130x_crtc_state that has a .data_array to store the pixels in the HW format (e.g: R1) and written to the panel. 2) struct ssd130x_plane_state that has a .buffer to store the pixels that are converted from the emulated XRGB used by the shadow-plane, to the HW pixel format. The (2) will be optional once Geert's R1 support lands. Now we are adding a third buffer so I wonder if should be part of one of these private state or not. I said that should be a field of struct ssd130x_plane_state in a previous email, but on a second thought it makes more sense if is a field of the struct ssd130x_crtc_state. That way the allocation will only be in ssd130x_crtc_atomic_check() and the release in the ssd130x_crtc_destroy_state(). If you do that on patch #2, then this patch #5 could be dropped. The reason why I added those private state structures is twofold: because the buffers are tied to the CRTC and planes and to show how a driver can maintain their own private atomic state. After all, one of my goals of this driver is to be used for educational purposes and provide a simple driver that people can use as a reference. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v2 2/5] drm/format-helper: Pass xfrm buffer to format-conversion helpers
Thomas Zimmermann writes: Hello Thomas, > Pass an instance of struct drm_xfrm_buf to DRM's format conversion > helpers. Update all callers. Drivers will later be able to keep this > cache across display updates. > > Signed-off-by: Thomas Zimmermann > Cc: Noralf Trønnes > Cc: Javier Martinez Canillas > Cc: Gerd Hoffmann > Cc: David Lechner > --- [...] > diff --git a/drivers/gpu/drm/solomon/ssd130x.c > b/drivers/gpu/drm/solomon/ssd130x.c > index 5a80b228d18ca..d11079733bc0e 100644 > --- a/drivers/gpu/drm/solomon/ssd130x.c > +++ b/drivers/gpu/drm/solomon/ssd130x.c > @@ -571,6 +571,7 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state > *state, > struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev); > unsigned int page_height = ssd130x->device_info->page_height; > struct ssd130x_plane_state *ssd130x_state = > to_ssd130x_plane_state(state); > + struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT; I would prefer if this structure is a field of struct ssd130x_plane_state. Since ssd130x_primary_plane_helper_atomic_check() zero allocates that, it will have the same initial values as set by DRM_XFRM_BUF_INIT. Alternatively you can do a drmm_xfrm_buf_init() + drm_xfrm_buf_reserve() in ssd130x_primary_plane_helper_atomic_check(). > u8 *buf = ssd130x_state->buffer; and struct drm_xfrm_buf *xfrm = _state->xfrm; > struct iosys_map dst; > unsigned int dst_pitch; > @@ -587,12 +588,14 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state > *state, > return ret; > > iosys_map_set_vaddr(, buf); > - drm_fb_xrgb_to_mono(, _pitch, vmap, fb, rect); > + drm_fb_xrgb_to_mono(, _pitch, vmap, fb, rect, ); > > drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE); > > ssd130x_update_rect(ssd130x, ssd130x_state, rect); > > + drm_xfrm_buf_release(); > + and you can release it in ssd130x_primary_plane_destroy_state(). -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195
Il 28/09/23 18:49, Conor Dooley ha scritto:
On Thu, Sep 28, 2023 at 02:52:23AM +, Moudy Ho (何宗原) wrote:
On Wed, 2023-09-27 at 10:47 +0100, Conor Dooley wrote:
On Wed, Sep 27, 2023 at 07:19:28AM +, Moudy Ho (何宗原) wrote:
On Fri, 2023-09-22 at 16:51 +0100, Conor Dooley wrote:
On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
Add a compatible string for the COLOR block in MediaTek
MT8195
that
is controlled by MDP3.
Signed-off-by: Moudy Ho
---
.../devicetree/bindings/display/mediatek/mediatek,color.yaml
| 1 +
1 file changed, 1 insertion(+)
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek
,col
or.yaml
b/Documentation/devicetree/bindings/display/mediatek/mediatek
,col
or.yaml
index f21e44092043..b886ca0d89ea 100644
---
a/Documentation/devicetree/bindings/display/mediatek/mediatek
,col
or.yaml
+++
b/Documentation/devicetree/bindings/display/mediatek/mediatek
,col
or.yaml
@@ -26,6 +26,7 @@ properties:
- mediatek,mt2701-disp-color
- mediatek,mt8167-disp-color
- mediatek,mt8173-disp-color
+ - mediatek,mt8195-mdp3-color
How come this one is a "mdp3" not a "disp"?
I don't know what mdp3 means & googling gives me no answers.
What's
the
"disp" one controlled by, since it isn't controlled by mdp3?
Mediatek's Media Data Path ver.3 (MDP3) is associated with MMSYS
and
acts as an independent driver that operates between VDEC and DISP.
By controlling multiple components, it carries out tasks like
converting color formats, resizing, and applying specific Picture
Quality (PQ) effects.
The driver can be found at "driver/media/platform/mediatek/mdp3".
Since the same hardware components are configured in both MDP3 and
DISP, considering previous discussions, I attemped to integrate
into a
single binding, named after the controlling user.
I'm still kinda struggling to understand this. Do you mean that the
hardware can be controlled by either of the disp and mdp3 drivers,
and
a compatible containing "disp" would use one driver, and one
containing
"mdp3" would use another?
Sorry for any confusion caused by the software information. In the
video pipeline, after decoding, the data flows sequentially through two
subsystems: MDP and DISP. Each subsystems has multiple IPs, with some
serving the same functionality as COLOR mentioned here. However, these
IPs cannot be controlled by different subsystems. Therefore, I included
the name of the subsystem after SoC to identify the configuration's
location. Is this approach feasible?
I'll have to leave things to the likes of Laurent to comment here I
think. I don't understand this hardware well enough to have a useful
opinion. It would seem like a different part of the datapath is a
different device that should be documented separately, but I don't know
enough to say for sure, sorry.
Hardware speaking, it's not a different device: those all reside in the
same block, except they are configured to route their I/O *either* to the
display pipeline, *or* to the MDP3 pipeline.
I would agree though in that this could be more flexible, as in, not
having a requirement to say "mdp3" or "disp", and managing the COLOR
blocks generically and letting the drivers to choose the actual path
transparently from what the devicetree compatible is, but there's no
practical point in doing this in the end, because there is an enough
number of (for example, COLOR) blocks such that one can be completely
reserved to MDP3 and one completely reserved to DISP.
So, we don't *need* this flexibility, but would be nice to have for
different (unexistant, basically) usecases...
The thing is, if we go for the maximum flexibility, the drawback is
that we'd see a number of nodes like
shared_block: something@somewhere {
compatible = "mediatek,something";
}
mdp3: dma-controller@14001000 {
..
mediatek,color = <>;
mediatek,stitch = <>;
mediatek,hdr = <>;
mediatek,aal = <>;
long list of another 10 components
}
display: something@somewhere {
..
an even longer list than the MDP3 one
}
...or perhaps even a graph, which is even longer in the end.
I'm not against this kind of structure, but I wonder if it's worth it.
Cheers,
Angelo
Re: [Intel-gfx] [PATCH v7 1/4] drm/i915: Introduce intel_gt_mcr_lock_sanitize()
On 9/28/2023 11:42 PM, Andrzej Hajda wrote:
On 28.09.2023 15:00, Nirmoy Das wrote:
Implement intel_gt_mcr_lock_sanitize() to provide a mechanism
for cleaning the steer semaphore when absolutely necessary.
v2: remove unnecessary lock(Andi, Matt)
improve the kernel doc(Matt)
s/intel_gt_mcr_lock_clear/intel_gt_mcr_lock_sanitize
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 22 ++
drivers/gpu/drm/i915/gt/intel_gt_mcr.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
index bf4a933de03a..326c2ed1d99b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
@@ -419,6 +419,28 @@ void intel_gt_mcr_unlock(struct intel_gt *gt,
unsigned long flags)
intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1);
}
+/**
+ * intel_gt_mcr_lock_sanitize - Sanitize MCR steering lock
+ * @gt: GT structure
+ *
+ * This will be used to sanitize the initial status of the hardware
lock
+ * during driver load and resume since there won't be any concurrent
access
+ * from other agents at those times, but it's possible that boot
firmware
+ * may have left the lock in a bad state.
+ *
+ */
+void intel_gt_mcr_lock_sanitize(struct intel_gt *gt)
+{
+ /*
+ * This gets called at load/resume time, so we shouldn't be
+ * racing with other driver threads grabbing the mcr lock.
+ */
+ lockdep_assert_not_held(>mcr_lock);
+
+ if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70))
+ intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1);
I wonder if it wouldn't be useful to check and report if it is locked
before unconditional release, no strong feelings.
Not so useful for user but may be as debug log if we need.
Reviewed-by: Andrzej Hajda
Thanks,
Nirmoy
Regards
Andrzej
+}
+
/**
* intel_gt_mcr_read - read a specific instance of an MCR register
* @gt: GT structure
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
index 41684495b7da..01ac565a56a4 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
@@ -11,6 +11,7 @@
void intel_gt_mcr_init(struct intel_gt *gt);
void intel_gt_mcr_lock(struct intel_gt *gt, unsigned long *flags);
void intel_gt_mcr_unlock(struct intel_gt *gt, unsigned long flags);
+void intel_gt_mcr_lock_sanitize(struct intel_gt *gt);
u32 intel_gt_mcr_read(struct intel_gt *gt,
i915_mcr_reg_t reg,
Re: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8
On 28/09/2023 20:53, Felix Kuehling wrote: On 2023-09-28 11:38, Shashank Sharma wrote: Hello Felix, Mukul, On 28/09/2023 17:30, Felix Kuehling wrote: On 2023-09-28 10:30, Joshi, Mukul wrote: [AMD Official Use Only - General] -Original Message- From: Yadav, Arvind Sent: Thursday, September 28, 2023 5:54 AM To: Koenig, Christian ; Deucher, Alexander ; Sharma, Shashank ; Kuehling, Felix ; Joshi, Mukul ; Pan, Xinhui ; airl...@gmail.com; dan...@ffwll.ch Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- ker...@vger.kernel.org; Yadav, Arvind ; Koenig, Christian Subject: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8 This patch is to adjust the absolute doorbell offset against the doorbell id considering the doorbell size of 32/64 bit. v2: - Addressed the review comment from Felix. Cc: Christian Koenig Cc: Alex Deucher Signed-off-by: Shashank Sharma Signed-off-by: Arvind Yadav --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index 0d3d538b64eb..c54c4392d26e 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -407,7 +407,14 @@ static int allocate_doorbell(struct qcm_process_device *qpd, q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev- adev, qpd- proc_doorbells, - q- doorbell_id); + 0); + It looks like amdgpu_doorbell_index_on_bar() works only for 64-bit doorbells. Shouldn't it work for both 32-bit and 64-bit doorbells considering this is common doorbell manager code? Yes, You are right that the calculations to find a particular doorbell in the doorbell page considers a doorbell width of 64-bit. I could see this argument going either way. KFD is the only one that cares about managing doorbells for user mode queues on GFXv8 GPUs. This is not a use case that amdgpu cares about. So I'm OK with KFD doing its own address calculations to make sure doorbells continue to work on GFXv8. It may not be worth adding complexity to the common doorbell manager code to support legacy GPUs with 32-bit doorbells. I was thinking about adding an additional input parameter which will indicate if the doorbell width is 32-bit vs 64-bit (like is_doorbell_64_bit), and doorbell manager can alter the multiplier while calculating the final offset. Please let me know if that will work for both the cases. Yes, that would work for KFD because we already have the doorbell size in our device-info structure. Instead of making it a boolean flag, you could make it a doorbell_size parameter, in byte or dword units to simplify the pointer math. Sounds good, will do that and send an update. - Shashank Regards, Felix - Shashank Regards, Felix Thanks, Mukul + /* Adjust the absolute doorbell offset against the doorbell id considering + * the doorbell size of 32/64 bit. + */ + q->properties.doorbell_off += q->doorbell_id * + dev->kfd->device_info.doorbell_size / 4; + return 0; } -- 2.34.1
Re: [PATCH v7 1/4] drm/i915: Introduce intel_gt_mcr_lock_sanitize()
Thanks reviewing this series. Merged it in gt-next so hopefully we have
bit greener CI for MTL now.
Regards,
Nirmoy
On 9/28/2023 3:00 PM, Nirmoy Das wrote:
Implement intel_gt_mcr_lock_sanitize() to provide a mechanism
for cleaning the steer semaphore when absolutely necessary.
v2: remove unnecessary lock(Andi, Matt)
improve the kernel doc(Matt)
s/intel_gt_mcr_lock_clear/intel_gt_mcr_lock_sanitize
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 22 ++
drivers/gpu/drm/i915/gt/intel_gt_mcr.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
index bf4a933de03a..326c2ed1d99b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
@@ -419,6 +419,28 @@ void intel_gt_mcr_unlock(struct intel_gt *gt, unsigned
long flags)
intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1);
}
+/**
+ * intel_gt_mcr_lock_sanitize - Sanitize MCR steering lock
+ * @gt: GT structure
+ *
+ * This will be used to sanitize the initial status of the hardware lock
+ * during driver load and resume since there won't be any concurrent access
+ * from other agents at those times, but it's possible that boot firmware
+ * may have left the lock in a bad state.
+ *
+ */
+void intel_gt_mcr_lock_sanitize(struct intel_gt *gt)
+{
+ /*
+* This gets called at load/resume time, so we shouldn't be
+* racing with other driver threads grabbing the mcr lock.
+*/
+ lockdep_assert_not_held(>mcr_lock);
+
+ if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70))
+ intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1);
+}
+
/**
* intel_gt_mcr_read - read a specific instance of an MCR register
* @gt: GT structure
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
index 41684495b7da..01ac565a56a4 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h
@@ -11,6 +11,7 @@
void intel_gt_mcr_init(struct intel_gt *gt);
void intel_gt_mcr_lock(struct intel_gt *gt, unsigned long *flags);
void intel_gt_mcr_unlock(struct intel_gt *gt, unsigned long flags);
+void intel_gt_mcr_lock_sanitize(struct intel_gt *gt);
u32 intel_gt_mcr_read(struct intel_gt *gt,
i915_mcr_reg_t reg,
Re: [PATCH v2 1/5] drm/format-helper: Add struct drm_xfrm_buf to cache format conversion
Thomas Zimmermann writes:
> Hold temporary memory for format conversion in an instance of struct
> drm_xfrm_buf. Update internal helpers of DRM's format-conversion code
> accordingly. Drivers will later be able to keep this cache across
> display updates.
>
> Signed-off-by: Thomas Zimmermann
> ---
[...]
> +int drmm_xfrm_buf_init(struct drm_device *dev, struct drm_xfrm_buf *buf)
> +{
> + buf->mem = NULL;
> + buf->size = 0;
> + buf->preallocated = false;
> +
> + return drmm_add_action_or_reset(dev, drm_xfrm_buf_init_release, buf);
> +}
> +EXPORT_SYMBOL(drmm_xfrm_buf_init);
> +
Can we find a better name than xfrm? I know that this is what's used in
the internal drm_format_helper.c helpers but if we are exposing this to
drivers, then I think that the naming is not self explanatory.
> +/**
> + * drm_xfrm_buf_reserve - Allocates storage in an xfrm buffer
> + * @buf: The xfrm buffer
At least in the kernel-doc we can say "The buffer used for pixel format
conversion" or something along those lines.
[...]
> +/**
> + * struct drm_xfrm_buf - Stores transformation and conversion state
> + *
> + * DRM helpers for format conversion store temporary state in
> + * struct drm_xfrm_buf. The buffer's resources can be reused
And same here. Maybe struct drm_fmt_conversion_buf ?
Other than this nit, the patch looks good to me.
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
RE: [PATCH v1] drm/dp/mst: fix missing modeset unlock for MST port detect
On Fri, 29 Sep 2023, Ramya SR wrote: > Hi All , > > Please review the reply comment. Please read the responses you do get [1]. Please stop top-posting. Please fix your mail client. BR, Jani. [1] https://lore.kernel.org/r/877coak8o3@intel.com > > Regards, > Ramya SR > > -Original Message- > From: Ramya SR > Sent: Thursday, September 28, 2023 7:55 AM > To: 'Alex Deucher' ; 'imre.d...@intel.com' > > Cc: 'Lyude Paul' ; 'Jani Nikula' ; > 'Jeff Layton' ; 'linux-ker...@vger.kernel.org' > ; 'dri-devel@lists.freedesktop.org' > ; 'Wayne Lin' ; 'Alex > Deucher' ; Ramya SR (QUIC) > Subject: RE: [PATCH v1] drm/dp/mst: fix missing modeset unlock for MST port > detect > > Reminder. Please review the reply comment. > > Thanks and Regards, > Ramya SR > > -Original Message- > From: Ramya SR > Sent: Tuesday, September 26, 2023 4:12 PM > To: Alex Deucher ; imre.d...@intel.com > Cc: Lyude Paul ; Jani Nikula ; Jeff > Layton ; linux-ker...@vger.kernel.org; > dri-devel@lists.freedesktop.org; Wayne Lin ; Alex Deucher > ; Ramya SR (QUIC) > Subject: RE: [PATCH v1] drm/dp/mst: fix missing modeset unlock for MST port > detect > > > > -Original Message- > From: Alex Deucher > Sent: Monday, September 25, 2023 8:27 PM > To: imre.d...@intel.com > Cc: Lyude Paul ; Jani Nikula ; Jeff > Layton ; linux-ker...@vger.kernel.org; > dri-devel@lists.freedesktop.org; Wayne Lin ; Alex Deucher > ; Ramya SR (QUIC) > Subject: Re: [PATCH v1] drm/dp/mst: fix missing modeset unlock for MST port > detect > > WARNING: This email originated from outside of Qualcomm. Please be wary of > any links or attachments, and do not enable macros. > > On Fri, Sep 22, 2023 at 3:22 PM Imre Deak wrote: >> >> On Fri, Sep 22, 2023 at 03:02:23PM -0400, Lyude Paul wrote: >> > >> > Oh! wow thank you for catching this: >> > >> > Reviewed-by: Lyude Paul >> > >> > I will go and push this to drm-misc-next in just a moment >> > >> > On Fri, 2023-09-15 at 10:24 +0530, Ramya SR wrote: >> > > Modeset mutex unlock is missing in drm_dp_mst_detect_port function. >> > > This will lead to deadlock if calling the function multiple times >> > > in an atomic operation, for example, getting imultiple MST ports >> > > status for a DP MST bonding scenario. >> > > >> > > Signed-off-by: Ramya SR >> > > --- >> > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++- >> > > 1 file changed, 3 insertions(+), 1 deletion(-) >> > > >> > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c >> > > b/drivers/gpu/drm/display/drm_dp_mst_topology.c >> > > index ed96cfc..d6512c4 100644 >> > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c >> > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c >> > > @@ -4154,7 +4154,7 @@ drm_dp_mst_detect_port(struct drm_connector >> > > *connector, >> > > >> > > ret = drm_modeset_lock(>base.lock, ctx); >> > > if (ret) >> > > - goto out; >> > > + goto fail; >> > > >> > > ret = connector_status_disconnected; >> > > >> > > @@ -4181,6 +4181,8 @@ drm_dp_mst_detect_port(struct drm_connector >> > > *connector, >> > > break; >> > > } >> > > out: >> > > + drm_modeset_unlock(>base.lock); >> >> Isn't this supposed to be unlocked only by >> drm_helper_probe_detect_ctx() / drm_helper_probe_detect() ? > > Maybe adding a comment to that effect would be helpful for the future. > > Alex > >>this is different lock, above function mentioned is locking/unlocking the >>global connection_mutex, that is required always locked during the atomic >>>check/commit. Here we are talking about MST topology manager mutex >>"mgr->base.lock". > >>For normal non-bond MST, it's ok, the calling sequence for detecting >>MST connector status is dp_mst_connector_detect >>->mst->mst_fw_cbs->detect_port_ctx = dp_mst_detect_port >>->drm_dp_mst_detect_port (where mgr->base.lock is locked) > >> Then for the bond MST case, to figure out if the sibling connectors >> are also connected, so that the bonding is possible, we need detect >> two or more connectors >in single dp_mst_connector_detect call > >>dp_mst_connector_detect ->mst->mst_fw_cbs->detect_port_ctx = >>dp_mst_detect_port ->drm_dp_mst_detect_port (where mgr->base.lock is >>locked) dp_mst_find_sibling_connector >>->mst->mst_fw_cbs->detect_port_ctx = dp_mst_detect_port >>->drm_dp_mst_detect_port (blocked by mgr->base.lock) > >> >> > > +fail: >> > > drm_dp_mst_topology_put_port(port); >> > > return ret; >> > > } >> > >> > -- >> > Cheers, >> > Lyude Paul (she/her) >> > Software Engineer at Red Hat >> > -- Jani Nikula, Intel
[PATCH 4/8] drm/i915/audio : Consider fractional vdsc bpp while computing tu_data
From: Ankit Nautiyal MTL+ supports fractional compressed bits_per_pixel, with precision of 1/16. This compressed bpp is stored in U6.4 format. Accommodate the precision during calculation of transfer unit data for hblank_early calculation. v2: -Fixed tu_data calculation while dealing with U6.4 format. (Stan) Signed-off-by: Ankit Nautiyal Reviewed-by: Suraj Kandpal Reviewed-by: Sui Jingfeng --- drivers/gpu/drm/i915/display/intel_audio.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index 4f1db1581316..3b08be54ce4f 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -522,25 +522,25 @@ static unsigned int calc_hblank_early_prog(struct intel_encoder *encoder, unsigned int link_clks_available, link_clks_required; unsigned int tu_data, tu_line, link_clks_active; unsigned int h_active, h_total, hblank_delta, pixel_clk; - unsigned int fec_coeff, cdclk, vdsc_bpp; + unsigned int fec_coeff, cdclk, vdsc_bppx16; unsigned int link_clk, lanes; unsigned int hblank_rise; h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay; h_total = crtc_state->hw.adjusted_mode.crtc_htotal; pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock; - vdsc_bpp = intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16); + vdsc_bppx16 = crtc_state->dsc.compressed_bpp_x16; cdclk = i915->display.cdclk.hw.cdclk; /* fec= 0.972261, using rounding multiplier of 100 */ fec_coeff = 972261; link_clk = crtc_state->port_clock; lanes = crtc_state->lane_count; - drm_dbg_kms(>drm, "h_active = %u link_clk = %u :" - "lanes = %u vdsc_bpp = %u cdclk = %u\n", - h_active, link_clk, lanes, vdsc_bpp, cdclk); + drm_dbg_kms(>drm, + "h_active = %u link_clk = %u : lanes = %u vdsc_bppx16 = %u cdclk = %u\n", + h_active, link_clk, lanes, vdsc_bppx16, cdclk); - if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bpp || !cdclk)) + if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bppx16 || !cdclk)) return 0; link_clks_available = (h_total - h_active) * link_clk / pixel_clk - 28; @@ -552,8 +552,8 @@ static unsigned int calc_hblank_early_prog(struct intel_encoder *encoder, hblank_delta = DIV64_U64_ROUND_UP(mul_u32_u32(5 * (link_clk + cdclk), pixel_clk), mul_u32_u32(link_clk, cdclk)); - tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bpp * 8, 100), - mul_u32_u32(link_clk * lanes, fec_coeff)); + tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bppx16 * 8, 100), + mul_u32_u32(link_clk * lanes * 16, fec_coeff)); tu_line = div64_u64(h_active * mul_u32_u32(link_clk, fec_coeff), mul_u32_u32(64 * pixel_clk, 100)); link_clks_active = (tu_line - 1) * 64 + tu_data; -- 2.25.1
[PATCH 5/8] drm/i915/dsc/mtl: Add support for fractional bpp
From: Vandita Kulkarni
Consider the fractional bpp while reading the qp values.
v2: Use helpers for fractional, integral bits of bits_per_pixel. (Suraj)
Signed-off-by: Vandita Kulkarni
Signed-off-by: Ankit Nautiyal
Reviewed-by: Suraj Kandpal
Reviewed-by: Sui Jingfeng
---
.../gpu/drm/i915/display/intel_qp_tables.c| 3 ---
drivers/gpu/drm/i915/display/intel_vdsc.c | 25 +++
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_qp_tables.c
b/drivers/gpu/drm/i915/display/intel_qp_tables.c
index 543cdc46aa1d..600c815e37e4 100644
--- a/drivers/gpu/drm/i915/display/intel_qp_tables.c
+++ b/drivers/gpu/drm/i915/display/intel_qp_tables.c
@@ -34,9 +34,6 @@
* These qp tables are as per the C model
* and it has the rows pointing to bpps which increment
* in steps of 0.5
- * We do not support fractional bpps as of today,
- * hence we would skip the fractional bpps during
- * our references for qp calclulations.
*/
static const u8
rc_range_minqp444_8bpc[DSC_NUM_BUF_RANGES][RC_RANGE_QP444_8BPC_MAX_NUM_BPP] = {
{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 142c886f4776..e9f90c2c2ec4 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -78,8 +78,8 @@ intel_vdsc_set_min_max_qp(struct drm_dsc_config *vdsc_cfg,
int buf,
static void
calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
{
+ int bpp = intel_fractional_bpp_from_x16(vdsc_cfg->bits_per_pixel);
int bpc = vdsc_cfg->bits_per_component;
- int bpp = vdsc_cfg->bits_per_pixel >> 4;
int qp_bpc_modifier = (bpc - 8) * 2;
int uncompressed_bpg_rate;
int first_line_bpg_offset;
@@ -149,7 +149,13 @@ calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
static const s8 ofs_und8[] = {
10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12,
-12
};
-
+ /*
+* For 420 format since bits_per_pixel (bpp) is set to target
bpp * 2,
+* QP table values for target bpp 4.0 to 4.4375 (rounded to
4.0) are
+* actually for bpp 8 to 8.875 (rounded to 4.0 * 2 i.e 8).
+* Similarly values for target bpp 4.5 to 4.8375 (rounded to
4.5)
+* are for bpp 9 to 9.875 (rounded to 4.5 * 2 i.e 9), and so on.
+*/
bpp_i = bpp - 8;
for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
u8 range_bpg_offset;
@@ -179,6 +185,9 @@ calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
range_bpg_offset & DSC_RANGE_BPG_OFFSET_MASK;
}
} else {
+ /* fractional bpp part * 1 (for precision up to 4 decimal
places) */
+ int fractional_bits =
intel_fractional_bpp_decimal(vdsc_cfg->bits_per_pixel);
+
static const s8 ofs_und6[] = {
0, -2, -2, -4, -6, -6, -8, -8, -8, -10, -10, -12, -12,
-12, -12
};
@@ -192,7 +201,14 @@ calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12,
-12
};
- bpp_i = (2 * (bpp - 6));
+ /*
+* QP table rows have values in increment of 0.5.
+* So 6.0 bpp to 6.4375 will have index 0, 6.5 to 6.9375 will
have index 1,
+* and so on.
+* 0.5 fractional part with 4 decimal precision becomes 5000
+*/
+ bpp_i = ((bpp - 6) + (fractional_bits < 5000 ? 0 : 1));
+
for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
u8 range_bpg_offset;
@@ -280,8 +296,7 @@ int intel_dsc_compute_params(struct intel_crtc_state
*pipe_config)
/* Gen 11 does not support VBR */
vdsc_cfg->vbr_enable = false;
- /* Gen 11 only supports integral values of bpp */
- vdsc_cfg->bits_per_pixel = compressed_bpp << 4;
+ vdsc_cfg->bits_per_pixel = pipe_config->dsc.compressed_bpp_x16;
/*
* According to DSC 1.2 specs in Section 4.1 if native_420 is set
--
2.25.1
[PATCH 8/8] drm/i915/dsc: Allow DSC only with fractional bpp when forced from debugfs
From: Swati Sharma
If force_dsc_fractional_bpp_en is set through debugfs allow DSC iff
compressed bpp is fractional. Continue if the computed compressed bpp
turns out to be a integer.
v2:
-Use helpers for fractional, integral bits of bits_per_pixel. (Suraj)
-Fix comment (Suraj)
Signed-off-by: Swati Sharma
Reviewed-by: Suraj Kandpal
Reviewed-by: Sui Jingfeng
---
drivers/gpu/drm/i915/display/intel_dp.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index bc4ea1c21562..a9e1a89a2804 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1908,6 +1908,9 @@ xelpd_dsc_compute_link_config(struct intel_dp *intel_dp,
for (compressed_bppx16 = dsc_max_bpp;
compressed_bppx16 >= dsc_min_bpp;
compressed_bppx16 -= bppx16_step) {
+ if (intel_dp->force_dsc_fractional_bpp_en &&
+ !intel_fractional_bpp_decimal(compressed_bppx16))
+ continue;
ret = dsc_compute_link_config(intel_dp,
pipe_config,
limits,
@@ -1915,6 +1918,10 @@ xelpd_dsc_compute_link_config(struct intel_dp *intel_dp,
timeslots);
if (ret == 0) {
pipe_config->dsc.compressed_bpp_x16 = compressed_bppx16;
+ if (intel_dp->force_dsc_fractional_bpp_en &&
+ intel_fractional_bpp_decimal(compressed_bppx16))
+ drm_dbg_kms(>drm, "Forcing DSC fractional
bpp\n");
+
return 0;
}
}
--
2.25.1
[PATCH 7/8] drm/i915/dsc: Add debugfs entry to validate DSC fractional bpp
From: Swati Sharma
DSC_Sink_BPP_Precision entry is added to i915_dsc_fec_support_show
to depict sink's precision.
Also, new debugfs entry is created to enforce fractional bpp.
If Force_DSC_Fractional_BPP_en is set then while iterating over
output bpp with fractional step size we will continue if output_bpp is
computed as integer. With this approach, we will be able to validate
DSC with fractional bpp.
v2:
Add drm_modeset_unlock to new line(Suraj)
Signed-off-by: Swati Sharma
Signed-off-by: Ankit Nautiyal
Signed-off-by: Mitul Golani
Reviewed-by: Suraj Kandpal
Reviewed-by: Sui Jingfeng
---
.../drm/i915/display/intel_display_debugfs.c | 84 +++
.../drm/i915/display/intel_display_types.h| 1 +
2 files changed, 85 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index f05b52381a83..8de41c820eed 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1244,6 +1244,8 @@ static int i915_dsc_fec_support_show(struct seq_file *m,
void *data)
DP_DSC_YCbCr420_Native)),
str_yes_no(drm_dp_dsc_sink_supports_format(intel_dp->dsc_dpcd,
DP_DSC_YCbCr444)));
+ seq_printf(m, "DSC_Sink_BPP_Precision: %d\n",
+ drm_dp_dsc_sink_bpp_incr(intel_dp->dsc_dpcd));
seq_printf(m, "Force_DSC_Enable: %s\n",
str_yes_no(intel_dp->force_dsc_en));
if (!intel_dp_is_edp(intel_dp))
@@ -1436,6 +1438,85 @@ static const struct file_operations
i915_dsc_output_format_fops = {
.write = i915_dsc_output_format_write
};
+static int i915_dsc_fractional_bpp_show(struct seq_file *m, void *data)
+{
+ struct drm_connector *connector = m->private;
+ struct drm_device *dev = connector->dev;
+ struct drm_crtc *crtc;
+ struct intel_dp *intel_dp;
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+ struct intel_encoder *encoder = intel_attached_encoder(intel_connector);
+ int ret;
+
+ if (!encoder)
+ return -ENODEV;
+
+ ret =
drm_modeset_lock_single_interruptible(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
+ crtc = connector->state->crtc;
+ if (connector->status != connector_status_connected || !crtc) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ intel_dp = intel_attached_dp(intel_connector);
+ seq_printf(m, "Force_DSC_Fractional_BPP_Enable: %s\n",
+ str_yes_no(intel_dp->force_dsc_fractional_bpp_en));
+
+out:
+ drm_modeset_unlock(>mode_config.connection_mutex);
+
+ return ret;
+}
+
+static ssize_t i915_dsc_fractional_bpp_write(struct file *file,
+const char __user *ubuf,
+size_t len, loff_t *offp)
+{
+ struct drm_connector *connector =
+ ((struct seq_file *)file->private_data)->private;
+ struct intel_encoder *encoder =
intel_attached_encoder(to_intel_connector(connector));
+ struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+ struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+ bool dsc_fractional_bpp_enable = false;
+ int ret;
+
+ if (len == 0)
+ return 0;
+
+ drm_dbg(>drm,
+ "Copied %zu bytes from user to force fractional bpp for DSC\n",
len);
+
+ ret = kstrtobool_from_user(ubuf, len, _fractional_bpp_enable);
+ if (ret < 0)
+ return ret;
+
+ drm_dbg(>drm, "Got %s for DSC Fractional BPP Enable\n",
+ (dsc_fractional_bpp_enable) ? "true" : "false");
+ intel_dp->force_dsc_fractional_bpp_en = dsc_fractional_bpp_enable;
+
+ *offp += len;
+
+ return len;
+}
+
+static int i915_dsc_fractional_bpp_open(struct inode *inode,
+ struct file *file)
+{
+ return single_open(file, i915_dsc_fractional_bpp_show,
inode->i_private);
+}
+
+static const struct file_operations i915_dsc_fractional_bpp_fops = {
+ .owner = THIS_MODULE,
+ .open = i915_dsc_fractional_bpp_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+ .write = i915_dsc_fractional_bpp_write
+};
+
/*
* Returns the Current CRTC's bpc.
* Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
@@ -1513,6 +1594,9 @@ void intel_connector_debugfs_add(struct intel_connector
*intel_connector)
debugfs_create_file("i915_dsc_output_format", 0644, root,
connector, _dsc_output_format_fops);
+
+
[PATCH 6/8] drm/i915/dp: Iterate over output bpp with fractional step size
From: Ankit Nautiyal
This patch adds support to iterate over compressed output bpp as per the
fractional step, supported by DP sink.
v2:
-Avoid ending up with compressed bpp, same as pipe bpp. (Stan)
Signed-off-by: Ankit Nautiyal
Reviewed-by: Suraj Kandpal
Reviewed-by: Sui Jingfeng
---
drivers/gpu/drm/i915/display/intel_dp.c | 38 +++--
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index c4cb2b763161..bc4ea1c21562 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1718,15 +1718,15 @@ static bool intel_dp_dsc_supports_format(struct
intel_dp *intel_dp,
return drm_dp_dsc_sink_supports_format(intel_dp->dsc_dpcd,
sink_dsc_format);
}
-static bool is_bw_sufficient_for_dsc_config(u16 compressed_bpp, u32 link_clock,
+static bool is_bw_sufficient_for_dsc_config(u16 compressed_bppx16, u32
link_clock,
u32 lane_count, u32 mode_clock,
enum intel_output_format
output_format,
int timeslots)
{
u32 available_bw, required_bw;
- available_bw = (link_clock * lane_count * timeslots) / 8;
- required_bw = compressed_bpp * (intel_dp_mode_to_fec_clock(mode_clock));
+ available_bw = (link_clock * lane_count * timeslots * 16) / 8;
+ required_bw = compressed_bppx16 *
(intel_dp_mode_to_fec_clock(mode_clock));
return available_bw > required_bw;
}
@@ -1734,7 +1734,7 @@ static bool is_bw_sufficient_for_dsc_config(u16
compressed_bpp, u32 link_clock,
static int dsc_compute_link_config(struct intel_dp *intel_dp,
struct intel_crtc_state *pipe_config,
struct link_config_limits *limits,
- u16 compressed_bpp,
+ u16 compressed_bppx16,
int timeslots)
{
const struct drm_display_mode *adjusted_mode =
_config->hw.adjusted_mode;
@@ -1749,8 +1749,8 @@ static int dsc_compute_link_config(struct intel_dp
*intel_dp,
for (lane_count = limits->min_lane_count;
lane_count <= limits->max_lane_count;
lane_count <<= 1) {
- if (!is_bw_sufficient_for_dsc_config(compressed_bpp,
link_rate, lane_count,
-
adjusted_mode->clock,
+ if (!is_bw_sufficient_for_dsc_config(compressed_bppx16,
link_rate,
+lane_count,
adjusted_mode->clock,
pipe_config->output_format,
timeslots))
continue;
@@ -1863,7 +1863,7 @@ icl_dsc_compute_link_config(struct intel_dp *intel_dp,
ret = dsc_compute_link_config(intel_dp,
pipe_config,
limits,
- valid_dsc_bpp[i],
+ valid_dsc_bpp[i] << 4,
timeslots);
if (ret == 0) {
pipe_config->dsc.compressed_bpp_x16 =
@@ -1890,23 +1890,31 @@ xelpd_dsc_compute_link_config(struct intel_dp *intel_dp,
int pipe_bpp,
int timeslots)
{
- u16 compressed_bpp;
+ u8 bppx16_incr = drm_dp_dsc_sink_bpp_incr(intel_dp->dsc_dpcd);
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ u16 compressed_bppx16;
+ u8 bppx16_step;
int ret;
+ if (DISPLAY_VER(i915) < 14 || bppx16_incr <= 1)
+ bppx16_step = 16;
+ else
+ bppx16_step = 16 / bppx16_incr;
+
/* Compressed BPP should be less than the Input DSC bpp */
- dsc_max_bpp = min(dsc_max_bpp, pipe_bpp - 1);
+ dsc_max_bpp = min(dsc_max_bpp << 4, (pipe_bpp << 4) - bppx16_step);
+ dsc_min_bpp = dsc_min_bpp << 4;
- for (compressed_bpp = dsc_max_bpp;
-compressed_bpp >= dsc_min_bpp;
-compressed_bpp--) {
+ for (compressed_bppx16 = dsc_max_bpp;
+compressed_bppx16 >= dsc_min_bpp;
+compressed_bppx16 -= bppx16_step) {
ret = dsc_compute_link_config(intel_dp,
pipe_config,
limits,
- compressed_bpp,
+ compressed_bppx16,
timeslots);
if (ret == 0) {
-
[PATCH 2/8] drm/i915/display: Store compressed bpp in U6.4 format
From: Ankit Nautiyal
DSC parameter bits_per_pixel is stored in U6.4 format.
The 4 bits represent the fractional part of the bpp.
Currently we use compressed_bpp member of dsc structure to store
only the integral part of the bits_per_pixel.
To store the full bits_per_pixel along with the fractional part,
compressed_bpp is changed to store bpp in U6.4 formats. Intergral
part is retrieved by simply right shifting the member compressed_bpp by 4.
v2:
-Use to_bpp_int, to_bpp_frac_dec, to_bpp_x16 helpers while dealing
with compressed bpp. (Suraj)
-Fix comment styling. (Suraj)
v3:
-Add separate file for 6.4 fixed point helper(Jani, Nikula)
-Add comment for magic values(Suraj)
v4:
Fix checkpatch warnings caused by renaming(Suraj)
Signed-off-by: Ankit Nautiyal
Signed-off-by: Mitul Golani
Reviewed-by: Suraj Kandpal
Reviewed-by: Sui Jingfeng
---
drivers/gpu/drm/i915/display/icl_dsi.c| 11 +++---
drivers/gpu/drm/i915/display/intel_audio.c| 3 +-
drivers/gpu/drm/i915/display/intel_bios.c | 6 ++--
drivers/gpu/drm/i915/display/intel_cdclk.c| 6 ++--
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
.../drm/i915/display/intel_display_types.h| 3 +-
drivers/gpu/drm/i915/display/intel_dp.c | 33 ++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 26 --
.../i915/display/intel_fractional_helper.h| 36 +++
drivers/gpu/drm/i915/display/intel_vdsc.c | 5 +--
10 files changed, 93 insertions(+), 38 deletions(-)
create mode 100644 drivers/gpu/drm/i915/display/intel_fractional_helper.h
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
b/drivers/gpu/drm/i915/display/icl_dsi.c
index c4585e445198..77b73bd61076 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -43,6 +43,7 @@
#include "intel_de.h"
#include "intel_dsi.h"
#include "intel_dsi_vbt.h"
+#include "intel_fractional_helper.h"
#include "intel_panel.h"
#include "intel_vdsc.h"
#include "intel_vdsc_regs.h"
@@ -330,7 +331,7 @@ static int afe_clk(struct intel_encoder *encoder,
int bpp;
if (crtc_state->dsc.compression_enable)
- bpp = crtc_state->dsc.compressed_bpp;
+ bpp =
intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16);
else
bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
@@ -860,7 +861,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
* compressed and non-compressed bpp.
*/
if (crtc_state->dsc.compression_enable) {
- mul = crtc_state->dsc.compressed_bpp;
+ mul =
intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16);
div = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
}
@@ -884,7 +885,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder
*encoder,
int bpp, line_time_us, byte_clk_period_ns;
if (crtc_state->dsc.compression_enable)
- bpp = crtc_state->dsc.compressed_bpp;
+ bpp =
intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16);
else
bpp =
mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
@@ -1451,8 +1452,8 @@ static void gen11_dsi_get_timings(struct intel_encoder
*encoder,
struct drm_display_mode *adjusted_mode =
_config->hw.adjusted_mode;
- if (pipe_config->dsc.compressed_bpp) {
- int div = pipe_config->dsc.compressed_bpp;
+ if (pipe_config->dsc.compressed_bpp_x16) {
+ int div =
intel_fractional_bpp_from_x16(pipe_config->dsc.compressed_bpp_x16);
int mul = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
adjusted_mode->crtc_htotal =
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
b/drivers/gpu/drm/i915/display/intel_audio.c
index 19605264a35c..4f1db1581316 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -35,6 +35,7 @@
#include "intel_crtc.h"
#include "intel_de.h"
#include "intel_display_types.h"
+#include "intel_fractional_helper.h"
#include "intel_lpe_audio.h"
/**
@@ -528,7 +529,7 @@ static unsigned int calc_hblank_early_prog(struct
intel_encoder *encoder,
h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock;
- vdsc_bpp = crtc_state->dsc.compressed_bpp;
+ vdsc_bpp =
intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16);
cdclk = i915->display.cdclk.hw.cdclk;
/* fec= 0.972261, using rounding multiplier of 100 */
fec_coeff = 972261;
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
b/drivers/gpu/drm/i915/display/intel_bios.c
index 4e8f1e91bb08..616492a1a7ef 100644
---
[PATCH 3/8] drm/i915/display: Consider fractional vdsc bpp while computing m_n values
From: Ankit Nautiyal
MTL+ supports fractional compressed bits_per_pixel, with precision of
1/16. This compressed bpp is stored in U6.4 format.
Accommodate this precision while computing m_n values.
v1:
Replace the computation of 'data_clock' with 'data_clock =
DIV_ROUND_UP(data_clock, 16).' (Sui Jingfeng).
Signed-off-by: Ankit Nautiyal
Signed-off-by: Mitul Golani
Reviewed-by: Suraj Kandpal
Reviewed-by: Sui Jingfeng
---
drivers/gpu/drm/i915/display/intel_display.c | 6 +-
drivers/gpu/drm/i915/display/intel_display.h | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 5 +++--
drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 --
drivers/gpu/drm/i915/display/intel_fdi.c | 2 +-
5 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 01cc22e97460..44aea5a6a9c4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2398,10 +2398,14 @@ void
intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
int pixel_clock, int link_clock,
struct intel_link_m_n *m_n,
- bool fec_enable)
+ bool fec_enable,
+ bool is_dsc_fractional_bpp)
{
u32 data_clock = bits_per_pixel * pixel_clock;
+ if (is_dsc_fractional_bpp)
+ data_clock = DIV_ROUND_UP(data_clock, 16);
+
if (fec_enable)
data_clock = intel_dp_mode_to_fec_clock(data_clock);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index 0e5dffe8f018..08ecb07485fd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -396,7 +396,7 @@ u8 intel_calc_active_pipes(struct intel_atomic_state *state,
void intel_link_compute_m_n(u16 bpp, int nlanes,
int pixel_clock, int link_clock,
struct intel_link_m_n *m_n,
- bool fec_enable);
+ bool fec_enable, bool is_dsc_fractional_bpp);
u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
u32 pixel_format, u64 modifier);
enum drm_mode_status
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 15de7940a433..c4cb2b763161 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2656,7 +2656,7 @@ intel_dp_drrs_compute_config(struct intel_connector
*connector,
intel_link_compute_m_n(link_bpp, pipe_config->lane_count, pixel_clock,
pipe_config->port_clock, _config->dp_m2_n2,
- pipe_config->fec_enable);
+ pipe_config->fec_enable, false);
/* FIXME: abstract this better */
if (pipe_config->splitter.enable)
@@ -2838,7 +2838,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
adjusted_mode->crtc_clock,
pipe_config->port_clock,
_config->dp_m_n,
- pipe_config->fec_enable);
+ pipe_config->fec_enable,
+ pipe_config->dsc.compression_enable);
/* FIXME: abstract this better */
if (pipe_config->splitter.enable)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index cb189b930b5b..fc39f5681aa3 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -178,7 +178,8 @@ static int intel_dp_mst_compute_link_config(struct
intel_encoder *encoder,
adjusted_mode->crtc_clock,
crtc_state->port_clock,
_state->dp_m_n,
- crtc_state->fec_enable);
+ crtc_state->fec_enable,
+ false);
crtc_state->dp_m_n.tu = slots;
return 0;
@@ -278,7 +279,8 @@ static int intel_dp_dsc_mst_compute_link_config(struct
intel_encoder *encoder,
adjusted_mode->crtc_clock,
crtc_state->port_clock,
_state->dp_m_n,
- crtc_state->fec_enable);
+ crtc_state->fec_enable,
+ crtc_state->dsc.compression_enable);
crtc_state->dp_m_n.tu = slots;
return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c
b/drivers/gpu/drm/i915/display/intel_fdi.c
index e6429dfebe15..f9de59e8b638 100644
--- a/drivers/gpu/drm/i915/display/intel_fdi.c
+++ b/drivers/gpu/drm/i915/display/intel_fdi.c
@@ -339,7 +339,7 @@ int
[PATCH 0/8] Add DSC fractional bpp support
This patch series adds support for DSC fractional compressed bpp for MTL+. The series starts with some fixes, followed by patches that lay groundwork to iterate over valid compressed bpps to select the 'best' compressed bpp with optimal link configuration (taken from upstream series: https://patchwork.freedesktop.org/series/105200/). The later patches, add changes to accommodate compressed bpp with fractional part, including changes to QP calculations. To get the 'best' compressed bpp, we iterate over the valid compressed bpp values, but with fractional step size 1/16, 1/8, 1/4 or 1/2 as per sink support. The last 2 patches add support to depict DSC sink's fractional support, and debugfs to enforce use of fractional bpp, while choosing an appropriate compressed bpp. Ankit Nautiyal (5): drm/display/dp: Add helper function to get DSC bpp precision drm/i915/display: Store compressed bpp in U6.4 format drm/i915/display: Consider fractional vdsc bpp while computing m_n values drm/i915/audio : Consider fractional vdsc bpp while computing tu_data drm/i915/dp: Iterate over output bpp with fractional step size Swati Sharma (2): drm/i915/dsc: Add debugfs entry to validate DSC fractional bpp drm/i915/dsc: Allow DSC only with fractional bpp when forced from debugfs Vandita Kulkarni (1): drm/i915/dsc/mtl: Add support for fractional bpp drivers/gpu/drm/display/drm_dp_helper.c | 27 ++ drivers/gpu/drm/i915/display/icl_dsi.c| 11 +-- drivers/gpu/drm/i915/display/intel_audio.c| 17 ++-- drivers/gpu/drm/i915/display/intel_bios.c | 6 +- drivers/gpu/drm/i915/display/intel_cdclk.c| 6 +- drivers/gpu/drm/i915/display/intel_display.c | 8 +- drivers/gpu/drm/i915/display/intel_display.h | 2 +- .../drm/i915/display/intel_display_debugfs.c | 84 +++ .../drm/i915/display/intel_display_types.h| 4 +- drivers/gpu/drm/i915/display/intel_dp.c | 81 +++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 32 --- drivers/gpu/drm/i915/display/intel_fdi.c | 2 +- .../i915/display/intel_fractional_helper.h| 36 .../gpu/drm/i915/display/intel_qp_tables.c| 3 - drivers/gpu/drm/i915/display/intel_vdsc.c | 30 +-- include/drm/display/drm_dp_helper.h | 1 + 16 files changed, 276 insertions(+), 74 deletions(-) create mode 100644 drivers/gpu/drm/i915/display/intel_fractional_helper.h -- 2.25.1
[PATCH 1/8] drm/display/dp: Add helper function to get DSC bpp precision
From: Ankit Nautiyal
Add helper to get the DSC bits_per_pixel precision for the DP sink.
Signed-off-by: Ankit Nautiyal
Reviewed-by: Suraj Kandpal
Reviewed-by: Sui Jingfeng
Acked-by: Maxime Ripard
---
drivers/gpu/drm/display/drm_dp_helper.c | 27 +
include/drm/display/drm_dp_helper.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index 8a1b64c57dfd..5c23d5b8fc50 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2323,6 +2323,33 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct
drm_dp_desc *desc,
}
EXPORT_SYMBOL(drm_dp_read_desc);
+/**
+ * drm_dp_dsc_sink_bpp_incr() - Get bits per pixel increment
+ * @dsc_dpcd: DSC capabilities from DPCD
+ *
+ * Returns the bpp precision supported by the DP sink.
+ */
+u8 drm_dp_dsc_sink_bpp_incr(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
+{
+ u8 bpp_increment_dpcd = dsc_dpcd[DP_DSC_BITS_PER_PIXEL_INC -
DP_DSC_SUPPORT];
+
+ switch (bpp_increment_dpcd) {
+ case DP_DSC_BITS_PER_PIXEL_1_16:
+ return 16;
+ case DP_DSC_BITS_PER_PIXEL_1_8:
+ return 8;
+ case DP_DSC_BITS_PER_PIXEL_1_4:
+ return 4;
+ case DP_DSC_BITS_PER_PIXEL_1_2:
+ return 2;
+ case DP_DSC_BITS_PER_PIXEL_1_1:
+ return 1;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_dsc_sink_bpp_incr);
+
/**
* drm_dp_dsc_sink_max_slice_count() - Get the max slice count
* supported by the DSC sink.
diff --git a/include/drm/display/drm_dp_helper.h
b/include/drm/display/drm_dp_helper.h
index 3369104e2d25..6968d4d87931 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -164,6 +164,7 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
}
/* DP/eDP DSC support */
+u8 drm_dp_dsc_sink_bpp_incr(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
bool is_edp);
u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
--
2.25.1
Re: [PATCH/RFC 3/3] drm: Split drm_modeset_helper_vtables.h
Hi
Am 29.09.23 um 09:33 schrieb Geert Uytterhoeven:
Hi Thomas,
On Fri, Sep 29, 2023 at 9:11 AM Thomas Zimmermann wrote:
Am 28.09.23 um 17:32 schrieb Geert Uytterhoeven:
On Thu, Sep 28, 2023 at 3:59 PM Thomas Zimmermann wrote:
Am 28.09.23 um 14:16 schrieb Geert Uytterhoeven:
is the second largest header file in
the DRM subsystem, and declares helpers vtables for various DRM
components. Several vtables contain methods with the same name, and all
but one vtable do not fit on the screen, making it hard to navigate to
the actual method one is interested in.
Make it easier for the casual reviewer to keep track by splitting
in multiple header files, one per DRM
component.
I never liked this header either, but do we need new header files? Each
struct could be appended to the end of the regular header: struct
drm_plane_helper_funcs to drm_plane.h, drm_connector_helper_func to
drm_connector.h and so on.
That would work for me, too. But perhaps we want to maintain a clear
separation between core and helpers?
Note that moving the contents to *_helper.h would be another option,
drm_crtc_helper.h and drm_plane_helper.h already exist.
I've taken a closer look at the users of the _vtables header. There's
code in drm_atomic_helper.c or drm_probe_helper.c that invokes the
callback functions.
The drivers fill the pointers with code that often comes from other
helper modules. That code is in files like drm_plane_helper.c or
drm_crtc_helper.c. There header files are drm_plane_helper.h, etc.
In that context, the _vtables header makes sense, as it separates the
callers from the callees. Putting the structs into headers like
drm_plane_helper.h would move it to the callee side.
I suggest to leave the header as it is. The fallout to the code base
from refactoring seems worse than the current state.
To clarify: do you mean keeping the single big drm_modeset_helper_vtables.h,
or the split drm_*_helper_vtable.h set?
I meant to keep the single big drm_modeset_helper_vtables.h. It's not
nice to look at, but I think we might easily mess up the header
dependencies with any changes.
Best regards
Thomas
Thanks!
Signed-off-by: Geert Uytterhoeven
---
RFC, a future patch could replace inclusion of
by inclusion of one or more of the
new files, and reduce compilation time.
---
include/drm/drm_connector_helper_vtable.h | 364 +
include/drm/drm_crtc_helper_vtable.h| 483 ++
include/drm/drm_encoder_helper_vtable.h | 381 +
include/drm/drm_mode_config_helper_vtable.h | 97 ++
include/drm/drm_modeset_helper_vtables.h| 1466 +--
include/drm/drm_plane_helper_vtable.h | 297
6 files changed, 1627 insertions(+), 1461 deletions(-)
create mode 100644 include/drm/drm_connector_helper_vtable.h
create mode 100644 include/drm/drm_crtc_helper_vtable.h
create mode 100644 include/drm/drm_encoder_helper_vtable.h
create mode 100644 include/drm/drm_mode_config_helper_vtable.h
create mode 100644 include/drm/drm_plane_helper_vtable.h
Gr{oetje,eeting}s,
Geert
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 07/32] drm/amd/display: document AMDGPU pre-defined transfer functions
On Thu, 28 Sep 2023 16:16:57 -0400
Harry Wentland wrote:
> On 2023-09-25 15:49, Melissa Wen wrote:
> > Brief documentation about pre-defined transfer function usage on AMD
> > display driver and standardized EOTFs and inverse EOTFs.
> >
> > v3:
> > - Document BT709 OETF (Pekka)
> > - Fix description of sRGB and pure power funcs (Pekka)
> >
> > Co-developed-by: Harry Wentland
> > Signed-off-by: Harry Wentland
> > Signed-off-by: Melissa Wen
> > ---
> > .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 39 +++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> > index d03bdb010e8b..14f9c02539c6 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> > @@ -85,6 +85,45 @@ void amdgpu_dm_init_color_mod(void)
> > }
> >
> > #ifdef AMD_PRIVATE_COLOR
> > +/* Pre-defined Transfer Functions (TF)
> > + *
> > + * AMD driver supports pre-defined mathematical functions for transferring
> > + * between encoded values and optical/linear space. Depending on HW color
> > caps,
> > + * ROMs and curves built by the AMD color module support these transforms.
> > + *
> > + * The driver-specific color implementation exposes properties for
> > pre-blending
> > + * degamma TF, shaper TF (before 3D LUT), and blend(dpp.ogam) TF and
> > + * post-blending regamma (mpc.ogam) TF. However, only pre-blending degamma
> > + * supports ROM curves. AMD color module uses pre-defined coefficients to
> > build
> > + * curves for the other blocks. What can be done by each color block is
> > + * described by struct dpp_color_capsand struct mpc_color_caps.
> > + *
> > + * AMD driver-specific color API exposes the following pre-defined transfer
> > + * functions:
> > + *
> > + * - Linear/Unity: linear/identity relationship between pixel value and
> > + * luminance value;
> > + * - Gamma 2.2, Gamma 2.4, Gamma 2.6: pure power functions;
> > + * - sRGB: 2.4: The piece-wise transfer function from IEC 61966-2-1:1999;
> > + * - BT.709: has a linear segment in the bottom part and then a power
> > function
> > + * with a 0.45 (~1/2.22) gamma for the rest of the range; standardized by
> > + * ITU-R BT.709-6;
> > + * - PQ (Perceptual Quantizer): used for HDR display, allows luminance
> > range
> > + * capability of 0 to 10,000 nits; standardized by SMPTE ST 2084.
> > + *
>
> I think it's important to highlight that the AMD color model is
> designed with an assumption that SDR (sRGB, BT.709, G2.2, etc.)
> peak white maps (normalized to 1.0 FP) to 80 nits in the PQ system.
> This has the implication that PQ EOTF (NL-to-L) maps to [0.0..125.0].
> 125.0 = 10,000 nits / 80 nits
>
> I think we'll want table or some other way describing this:
>
> (Using L to mean linear and NL to mean non-linear.)
>
> == sRGB, BT709, Gamma 2.x ==
> NL form is either UNORM or [0.0, 1.0]
> L form is [0.0, 1.0]
>
> Note that HDR multiplier can wide range beyond [0.0, 1.0].
> In practice this means that PQ TF is needed for any subsequent
> L-to-NL transforms.
>
> == PQ ==
> NL form is either UNORM or FP16 CCCS (Windows canonical composition color
> space, see [1])
> L form is [0.0, 125.0]
Hi,
what is [1]?
Thanks,
pq
> == Unity, Default ==
> NL form is either UNORM or FP16 CCCS
> L form is either [0.0, 1.0] (mapping from UNORM) or CCCS (mapping from CCCS
> FP16)
>
> Harry
>
> > + * In the driver-specific API, color block names attached to TF properties
> > + * suggest the intention regarding non-linear encoding pixel's luminance
> > + * values. As some newer encodings don't use gamma curve, we make encoding
> > and
> > + * decoding explicit by defining an enum list of transfer functions
> > supported
> > + * in terms of EOTF and inverse EOTF, where:
> > + *
> > + * - EOTF (electro-optical transfer function): is the transfer function to
> > go
> > + * from the encoded value to an optical (linear) value. De-gamma
> > functions
> > + * traditionally do this.
> > + * - Inverse EOTF (simply the inverse of the EOTF): is usually intended to
> > go
> > + * from an optical/linear space (which might have been used for blending)
> > + * back to the encoded values. Gamma functions traditionally do this.
> > + */
> > static const char * const
> > amdgpu_transfer_function_names[] = {
> > [AMDGPU_TRANSFER_FUNCTION_DEFAULT] = "Default",
>
>
pgpFKgKDEpxFb.pgp
Description: OpenPGP digital signature
Re: [PATCH/RFC 3/3] drm: Split drm_modeset_helper_vtables.h
Hi Thomas,
On Fri, Sep 29, 2023 at 9:11 AM Thomas Zimmermann wrote:
> Am 28.09.23 um 17:32 schrieb Geert Uytterhoeven:
> > On Thu, Sep 28, 2023 at 3:59 PM Thomas Zimmermann
> > wrote:
> >> Am 28.09.23 um 14:16 schrieb Geert Uytterhoeven:
> >>> is the second largest header file in
> >>> the DRM subsystem, and declares helpers vtables for various DRM
> >>> components. Several vtables contain methods with the same name, and all
> >>> but one vtable do not fit on the screen, making it hard to navigate to
> >>> the actual method one is interested in.
> >>>
> >>> Make it easier for the casual reviewer to keep track by splitting
> >>> in multiple header files, one per DRM
> >>> component.
> >>
> >> I never liked this header either, but do we need new header files? Each
> >> struct could be appended to the end of the regular header: struct
> >> drm_plane_helper_funcs to drm_plane.h, drm_connector_helper_func to
> >> drm_connector.h and so on.
> >
> > That would work for me, too. But perhaps we want to maintain a clear
> > separation between core and helpers?
> >
> > Note that moving the contents to *_helper.h would be another option,
> > drm_crtc_helper.h and drm_plane_helper.h already exist.
>
> I've taken a closer look at the users of the _vtables header. There's
> code in drm_atomic_helper.c or drm_probe_helper.c that invokes the
> callback functions.
>
> The drivers fill the pointers with code that often comes from other
> helper modules. That code is in files like drm_plane_helper.c or
> drm_crtc_helper.c. There header files are drm_plane_helper.h, etc.
>
> In that context, the _vtables header makes sense, as it separates the
> callers from the callees. Putting the structs into headers like
> drm_plane_helper.h would move it to the callee side.
>
> I suggest to leave the header as it is. The fallout to the code base
> from refactoring seems worse than the current state.
To clarify: do you mean keeping the single big drm_modeset_helper_vtables.h,
or the split drm_*_helper_vtable.h set?
Thanks!
> >>> Signed-off-by: Geert Uytterhoeven
> >>> ---
> >>> RFC, a future patch could replace inclusion of
> >>> by inclusion of one or more of the
> >>> new files, and reduce compilation time.
> >>> ---
> >>>include/drm/drm_connector_helper_vtable.h | 364 +
> >>>include/drm/drm_crtc_helper_vtable.h| 483 ++
> >>>include/drm/drm_encoder_helper_vtable.h | 381 +
> >>>include/drm/drm_mode_config_helper_vtable.h | 97 ++
> >>>include/drm/drm_modeset_helper_vtables.h| 1466 +--
> >>>include/drm/drm_plane_helper_vtable.h | 297
> >>>6 files changed, 1627 insertions(+), 1461 deletions(-)
> >>>create mode 100644 include/drm/drm_connector_helper_vtable.h
> >>>create mode 100644 include/drm/drm_crtc_helper_vtable.h
> >>>create mode 100644 include/drm/drm_encoder_helper_vtable.h
> >>>create mode 100644 include/drm/drm_mode_config_helper_vtable.h
> >>>create mode 100644 include/drm/drm_plane_helper_vtable.h
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 2/3] drm: Spelling s/preceeding/preceding/g
Am 28.09.23 um 14:16 schrieb Geert Uytterhoeven:
Fix misspellings of "preceding".
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
include/drm/drm_modeset_helper_vtables.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 71d3993971075eea..10aadd324cc370ee 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2373,10 +2373,10 @@ int drm_atomic_helper_setup_commit(struct
drm_atomic_state *state,
EXPORT_SYMBOL(drm_atomic_helper_setup_commit);
/**
- * drm_atomic_helper_wait_for_dependencies - wait for required preceeding
commits
+ * drm_atomic_helper_wait_for_dependencies - wait for required preceding
commits
* @old_state: atomic state object with old state structures
*
- * This function waits for all preceeding commits that touch the same CRTC as
+ * This function waits for all preceding commits that touch the same CRTC as
* @old_state to both be committed to the hardware (as signalled by
* drm_atomic_helper_commit_hw_done()) and executed by the hardware (as
signalled
* by calling drm_crtc_send_vblank_event() on the _crtc_state.event).
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index bbc516f313913254..91860372be6c064e 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -1464,7 +1464,7 @@ struct drm_mode_config_helper_funcs {
* swapped into the various state pointers. The passed in state
* therefore contains copies of the old/previous state. This hook should
* commit the new state into hardware. Note that the helpers have
-* already waited for preceeding atomic commits and fences, but drivers
+* already waited for preceding atomic commits and fences, but drivers
* can add more waiting calls at the start of their implementation, e.g.
* to wait for driver-internal request for implicit syncing, before
* starting to commit the update to the hardware.
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH 1/3] drm: Spelling s/hardward/hardware/g
Am 28.09.23 um 14:16 schrieb Geert Uytterhoeven:
Fix misspellings of "hardware".
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Thomas Zimmermann
---
include/drm/drm_bridge.h | 2 +-
include/drm/drm_modeset_helper_vtables.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index cfb7dcdb66c4b0b5..05b360a4357fed01 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -107,7 +107,7 @@ struct drm_bridge_funcs {
* Since this function is both called from the check phase of an atomic
* commit, and the mode validation in the probe paths it is not allowed
* to look at anything else but the passed-in mode, and validate it
-* against configuration-invariant hardward constraints. Any further
+* against configuration-invariant hardware constraints. Any further
* limits which depend upon the configuration can only be checked in
* @mode_fixup.
*
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index e3c3ac615909474b..bbc516f313913254 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -134,7 +134,7 @@ struct drm_crtc_helper_funcs {
* Since this function is both called from the check phase of an atomic
* commit, and the mode validation in the probe paths it is not allowed
* to look at anything else but the passed-in mode, and validate it
-* against configuration-invariant hardward constraints. Any further
+* against configuration-invariant hardware constraints. Any further
* limits which depend upon the configuration can only be checked in
* @mode_fixup or @atomic_check.
*
@@ -550,7 +550,7 @@ struct drm_encoder_helper_funcs {
* Since this function is both called from the check phase of an atomic
* commit, and the mode validation in the probe paths it is not allowed
* to look at anything else but the passed-in mode, and validate it
-* against configuration-invariant hardward constraints. Any further
+* against configuration-invariant hardware constraints. Any further
* limits which depend upon the configuration can only be checked in
* @mode_fixup or @atomic_check.
*
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH/RFC 3/3] drm: Split drm_modeset_helper_vtables.h
Hi
Am 28.09.23 um 17:32 schrieb Geert Uytterhoeven:
Hi Thomas,
On Thu, Sep 28, 2023 at 3:59 PM Thomas Zimmermann wrote:
Am 28.09.23 um 14:16 schrieb Geert Uytterhoeven:
is the second largest header file in
the DRM subsystem, and declares helpers vtables for various DRM
components. Several vtables contain methods with the same name, and all
but one vtable do not fit on the screen, making it hard to navigate to
the actual method one is interested in.
Make it easier for the casual reviewer to keep track by splitting
in multiple header files, one per DRM
component.
I never liked this header either, but do we need new header files? Each
struct could be appended to the end of the regular header: struct
drm_plane_helper_funcs to drm_plane.h, drm_connector_helper_func to
drm_connector.h and so on.
That would work for me, too. But perhaps we want to maintain a clear
separation between core and helpers?
Note that moving the contents to *_helper.h would be another option,
drm_crtc_helper.h and drm_plane_helper.h already exist.
I've taken a closer look at the users of the _vtables header. There's
code in drm_atomic_helper.c or drm_probe_helper.c that invokes the
callback functions.
The drivers fill the pointers with code that often comes from other
helper modules. That code is in files like drm_plane_helper.c or
drm_crtc_helper.c. There header files are drm_plane_helper.h, etc.
In that context, the _vtables header makes sense, as it separates the
callers from the callees. Putting the structs into headers like
drm_plane_helper.h would move it to the callee side.
I suggest to leave the header as it is. The fallout to the code base
from refactoring seems worse than the current state.
Best regards
Thomas
Signed-off-by: Geert Uytterhoeven
---
RFC, a future patch could replace inclusion of
by inclusion of one or more of the
new files, and reduce compilation time.
---
include/drm/drm_connector_helper_vtable.h | 364 +
include/drm/drm_crtc_helper_vtable.h| 483 ++
include/drm/drm_encoder_helper_vtable.h | 381 +
include/drm/drm_mode_config_helper_vtable.h | 97 ++
include/drm/drm_modeset_helper_vtables.h| 1466 +--
include/drm/drm_plane_helper_vtable.h | 297
6 files changed, 1627 insertions(+), 1461 deletions(-)
create mode 100644 include/drm/drm_connector_helper_vtable.h
create mode 100644 include/drm/drm_crtc_helper_vtable.h
create mode 100644 include/drm/drm_encoder_helper_vtable.h
create mode 100644 include/drm/drm_mode_config_helper_vtable.h
create mode 100644 include/drm/drm_plane_helper_vtable.h
Gr{oetje,eeting}s,
Geert
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH 3/3] drm/v3d: add brcm, 2712-v3d as a compatible V3D device
Hi Iago, additional to Maria's comments: Please keep the order of the compatible strings. Also you need to update the device tree binding before this patch: Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml Also make sure that the series is send to the maintainers, not just dri-devel by using scripts/get_maintainer.pl Best regards
Re: [RESEND PATCH v6 17/20] drm/mediatek: Support MT8188 Padding in display driver
[PATCH] drm: panel-orientation-quirks: Add quirk for One Mix 2S
The One Mix 2S is a mini laptop with a 1200x1920 portrait screen
mounted in a landscape oriented clamshell case. Because of the too
generic DMI strings this entry is also doing bios-date matching.
Signed-off-by: Kai Uwe Broulik
---
drivers/gpu/drm/drm_panel_orientation_quirks.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 0cb646cb04ee..cc9a9099faaf 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -38,6 +38,14 @@ static const struct drm_dmi_panel_orientation_data
gpd_micropc = {
.orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
};
+static const struct drm_dmi_panel_orientation_data gpd_onemix2s = {
+ .width = 1200,
+ .height = 1920,
+ .bios_dates = (const char * const []){ "03/04/2019",
+ NULL },
+ .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
static const struct drm_dmi_panel_orientation_data gpd_pocket = {
.width = 1200,
.height = 1920,
@@ -401,6 +409,14 @@ static const struct dmi_system_id orientation_data[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "LTH17"),
},
.driver_data = (void *)_rightside_up,
+ }, {/* One Mix 2S (generic strings, also match on bios date) */
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "Default string"),
+ },
+ .driver_data = (void *)_onemix2s,
},
{}
};
--
2.34.1
Re: [PATCH 5/9] dma-buf: heaps: mtk_sec_heap: Initialise tee session
Le 28/09/2023 à 19:48, Jeffrey Kardatzke a écrit : On Thu, Sep 28, 2023 at 1:30 AM Benjamin Gaignard wrote: Le 27/09/2023 à 20:56, Jeffrey Kardatzke a écrit : On Wed, Sep 27, 2023 at 8:18 AM Benjamin Gaignard wrote: Le 27/09/2023 à 15:46, Joakim Bech a écrit : On Mon, Sep 25, 2023 at 12:49:50PM +, Yong Wu (吴勇) wrote: On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno wrote: Il 11/09/23 04:30, Yong Wu ha scritto: The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't work here since this is not a platform driver, therefore initialise the TEE context/session while we allocate the first secure buffer. Signed-off-by: Yong Wu --- drivers/dma-buf/heaps/mtk_secure_heap.c | 61 + 1 file changed, 61 insertions(+) diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma- buf/heaps/mtk_secure_heap.c index bbf1c8dce23e..e3da33a3d083 100644 --- a/drivers/dma-buf/heaps/mtk_secure_heap.c +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c @@ -10,6 +10,12 @@ #include #include #include +#include +#include + +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- e41f1390d676" + Is this UUID the same for all SoCs and all TZ versions? Yes. It is the same for all SoCs and all TZ versions currently. That's good news! Is this UUID used in any userspace component? (example: Android HALs?) No. Userspace never use it. If userspace would like to allocate this secure buffer, it can achieve through the existing dmabuf IOCTL via /dev/dma_heap/mtk_svp node. In general I think as mentioned elsewhere in comments, that there isn't that much here that seems to be unique for MediaTek in this patch series, so I think it worth to see whether this whole patch set can be made more generic. Having said that, the UUID is always unique for a certain Trusted Application. So, it's not entirely true saying that the UUID is the same for all SoCs and all TrustZone versions. It might be true for a family of MediaTek devices and the TEE in use, but not generically. So, if we need to differentiate between different TA implementations, then we need different UUIDs. If it would be possible to make this patch set generic, then it sounds like a single UUID would be sufficient, but that would imply that all TA's supporting such a generic UUID would be implemented the same from an API point of view. Which also means that for example Trusted Application function ID's needs to be the same etc. Not impossible to achieve, but still not easy (different TEE follows different specifications) and it's not typically something we've done in the past. Unfortunately there is no standardized database of TA's describing what they implement and support. As an alternative, we could implement a query call in the TEE answering, "What UUID does your TA have that implements secure unmapped heap?". I.e., something that reminds of a lookup table. Then we wouldn't have to carry this in UAPI, DT or anywhere else. Joakim does a TA could offer a generic API and hide the hardware specific details (like kernel uAPI does for drivers) ? It would have to go through another layer (like the tee driver) to be a generic API. The main issue with TAs is that they have UUIDs you need to connect to and specific codes for each function; so we should abstract at a layer above where those exist in the dma-heap code. Aside that question I wonder what are the needs to perform a 'secure' playback. I have in mind 2 requirements: - secure memory regions, which means configure the hardware to ensure that only dedicated hardware blocks and read or write into it. - set hardware blocks in secure modes so they access to secure memory. Do you see something else ? This is more or less what is required, but this is out of scope for the Linux kernel since it can't be trusted to do these things...this is all done in firmware or the TEE itself. Yes kernel can't be trusted to do these things but know what we need could help to define a API for a generic TA. Just to brainstorm on mailing list: What about a TA API like TA_secure_memory_region() and TA_unsecure_memory_region() with parameters like: - device identifier (an ID or compatible string maybe) - memory region (physical address, size, offset) - requested access rights (read, write) and on kernel side a IOMMU driver because it basically have all this information already (device attachment, kernel map/unmap). In my mind it sound like a solution to limit the impact (new controls, new memory type) inside v4l2. Probably we won't need new heap either. All hardware dedicated implementations could live inside the TA which can offer a generic API. The main problem with that type of design is the limitations of TrustZone memory protection. Usually there is a limit to the number of regions you can define for memory protection (and
Re: [PATCH 1/3] drm/v3d: fix up register addresses for V3D 7.x
On 2023-09-28 15:03, Maira Canal wrote: > Hi Iago, > > On 9/28/23 08:45, Iago Toral Quiroga wrote: > > Please, add a commit message and your s-o-b to the patch. Here is a reference > on how to format your patches [1]. > > Also, please, run checkpatch on this patch and address the warnings. > > [1] https://docs.kernel.org/process/submitting-patches.html > >> --- >> drivers/gpu/drm/v3d/v3d_debugfs.c | 173 +- >> drivers/gpu/drm/v3d/v3d_gem.c | 3 + >> drivers/gpu/drm/v3d/v3d_irq.c | 47 >> drivers/gpu/drm/v3d/v3d_regs.h| 51 - >> drivers/gpu/drm/v3d/v3d_sched.c | 41 --- >> 5 files changed, 200 insertions(+), 115 deletions(-) >> >> diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c >> b/drivers/gpu/drm/v3d/v3d_debugfs.c >> index 330669f51fa7..90b2b5b2710c 100644 >> --- a/drivers/gpu/drm/v3d/v3d_debugfs.c >> +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c >> @@ -12,69 +12,83 @@ >> #include "v3d_drv.h" >> #include "v3d_regs.h" >> > > [...] > >> diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h >> index 3663e0d6bf76..9fbcbfedaae1 100644 >> --- a/drivers/gpu/drm/v3d/v3d_regs.h >> +++ b/drivers/gpu/drm/v3d/v3d_regs.h >> @@ -57,6 +57,7 @@ >> #define V3D_HUB_INT_MSK_STS0x0005c >> #define V3D_HUB_INT_MSK_SET0x00060 >> #define V3D_HUB_INT_MSK_CLR0x00064 >> +# define V3D_V7_HUB_INT_GMPV BIT(6) >> # define V3D_HUB_INT_MMU_WRV BIT(5) >> # define V3D_HUB_INT_MMU_PTI BIT(4) >> # define V3D_HUB_INT_MMU_CAP BIT(3) >> @@ -64,6 +65,7 @@ >> # define V3D_HUB_INT_TFUC BIT(1) >> # define V3D_HUB_INT_TFUF BIT(0) >> +/* GCA registers only exist in V3D < 41 */ >> #define V3D_GCA_CACHE_CTRL 0xc >> # define V3D_GCA_CACHE_CTRL_FLUSH BIT(0) >> @@ -87,6 +89,7 @@ >> # define V3D_TOP_GR_BRIDGE_SW_INIT_1_V3D_CLK_108_SW_INIT BIT(0) >> #define V3D_TFU_CS 0x00400 >> +#define V3D_V7_TFU_CS 0x00700 > > What do you think about something like > > #define V3D_TFU_CS(ver) (ver >= 71) ? 0x00700 : 0x00400 > > I believe that the code would get much cleaner and would avoid a bunch > of the if statements that you used. > > I would apply this format to all the new registers. Sure, that sounds good. Thanks! Iago > Best Regards, > - Maíra > >> /* Stops current job, empties input fifo. */ >> # define V3D_TFU_CS_TFURST BIT(31) >> # define V3D_TFU_CS_CVTCT_MASK V3D_MASK(23, 16) >> @@ -96,6 +99,7 @@ >> # define V3D_TFU_CS_BUSY BIT(0) >> #define V3D_TFU_SU 0x00404 >> +#define V3D_V7_TFU_SU 0x00704 >> /* Interrupt when FINTTHR input slots are free (0 = disabled) */ >> # define V3D_TFU_SU_FINTTHR_MASK V3D_MASK(13, 8) >> # define V3D_TFU_SU_FINTTHR_SHIFT 8 >> @@ -107,38 +111,53 @@ >> # define V3D_TFU_SU_THROTTLE_SHIFT 0 >> #define V3D_TFU_ICFG 0x00408 >> +#define V3D_V7_TFU_ICFG0x00708 >> /* Interrupt when the conversion is complete. */ >> # define V3D_TFU_ICFG_IOC BIT(0) >> /* Input Image Address */ >> #define V3D_TFU_IIA0x0040c >> +#define V3D_V7_TFU_IIA 0x0070c >> /* Input Chroma Address */ >> #define V3D_TFU_ICA0x00410 >> +#define V3D_V7_TFU_ICA 0x00710 >> /* Input Image Stride */ >> #define V3D_TFU_IIS0x00414 >> +#define V3D_V7_TFU_IIS 0x00714 >> /* Input Image U-Plane Address */ >> #define V3D_TFU_IUA0x00418 >> +#define V3D_V7_TFU_IUA 0x00718 >> +/* Image output config (VD 7.x only) */ >> +#define V3D_V7_TFU_IOC 0x0071c >> /* Output Image Address */ >> #define V3D_TFU_IOA0x0041c >> +#define V3D_V7_TFU_IOA 0x00720 >> /* Image Output Size */ >> #define V3D_TFU_IOS0x00420 >> +#define V3D_V7_TFU_IOS 0x00724 >> /* TFU YUV Coefficient 0 */ >> #define V3D_TFU_COEF0 0x00424 >> -/* Use these regs instead of the defaults. */ >> +#define V3D_V7_TFU_COEF0 0x00728 >> +/* Use these regs instead of the defaults
Re: [PATCH 2/3] drm/v3d: update UAPI to match user-space for V3D 7.x
On 2023-09-28 15:05, Maira Canal wrote:
> Hi Iago,
>
> On 9/28/23 08:45, Iago Toral Quiroga wrote:
>> V3D t.x takes a new parameter to configure TFU jobs that needs
>
> I believe t.x should be 7.x.
>
>> to be provided by user space.
>
> As I mentioned before, please, add your s-o-b.
>
>> ---
>> include/uapi/drm/v3d_drm.h | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
>> index 3dfc0af8756a..1a7d7a689de3 100644
>> --- a/include/uapi/drm/v3d_drm.h
>> +++ b/include/uapi/drm/v3d_drm.h
>> @@ -319,6 +319,11 @@ struct drm_v3d_submit_tfu {
>> /* Pointer to an array of ioctl extensions*/
>> __u64 extensions;
>> +
>> +struct {
>> +__u32 ioc;
>> +__u32 pad;
>> +} v71;
>
> Is there any possibility that the name of the struct could be more
> meaningful?
The v71 stands for the hardware version where this field was introduced,
so I am not sure how much more meaningful we can make it :)
The idea for this was to pack version-specific fields into structs named
vXX so that you can quickly tell in which version specific fields
started being relevant directly from the code without having to look for
documentation elsewhere. I don't have a better alternative for the name,
since the point is to make the version explicit, but I am open to
suggestions if you have any.
Of course, we can also get rid of the struct if you prefer that, but
then we should document explicitly that this field only applies to v71
hardware and we would lose the explicit versioning when accessing the
field from the code (unless we decide to add the v71 as a prefix or
suffix in the ioc field, but that is kind of the same thing).
Iago
>
> Best Regards,
> - Maíra
>
>> };
>> /* Submits a compute shader for dispatch. This job will block on any
