| Message ID | 20240209101412.1326176-5-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | XE HDCP Enablement | expand |
Hi Suraj, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-xe/drm-xe-next] [also build test WARNING on drm-tip/drm-tip] [cannot apply to drm-intel/for-linux-next drm-intel/for-linux-next-fixes linus/master v6.8-rc3 next-20240209] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Suraj-Kandpal/drm-i915-hdcp-Move-intel_hdcp_gsc_message-def-away-from-header-file/20240209-181915 base: https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next patch link: https://lore.kernel.org/r/20240209101412.1326176-5-suraj.kandpal%40intel.com patch subject: [PATCH 4/5] drm/xe/hdcp: Enable HDCP for XE config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20240210/202402100132.XIP3zz5i-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240210/202402100132.XIP3zz5i-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202402100132.XIP3zz5i-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/xe/xe_gsc_submit.c:50: warning: expecting prototype for xe_gsc_get_host_session_id(). Prototype was for xe_gsc_create_host_session_id() instead vim +50 drivers/gpu/drm/xe/xe_gsc_submit.c 42 43 /** 44 * xe_gsc_get_host_session_id - Creates a random 64 bit host_session id with 45 * bits 56-63 masked. 46 * 47 * Returns: random host_session_id which can be used to send messages to gsc cs 48 */ 49 u64 xe_gsc_create_host_session_id(void) > 50 { 51 u64 host_session_id; 52 53 get_random_bytes(&host_session_id, sizeof(u64)); 54 host_session_id &= ~HOST_SESSION_CLIENT_MASK; 55 return host_session_id; 56 } 57
On 2/9/2024 2:14 AM, Suraj Kandpal wrote: > Enable HDCP for Xe by defining functions which take care of > interaction of HDCP as a client with the GSC CS interface. > > --v2 > -add kfree at appropriate place [Daniele] > -remove useless define [Daniele] > -move host session logic to xe_gsc_submit.c [Daniele] > -call xe_gsc_check_and_update_pending directly in an if condition > [Daniele] > -use xe_device instead of drm_i915_private [Daniele] > > --v3 > -use xe prefix for newly exposed function [Daniele] > -remove client specific defines from intel_gsc_mtl_header [Daniele] > -add missing kfree() [Daniele] > -have NULL check for hdcp_message in finish function [Daniele] > -dont have too many variable declarations in the same line [Daniele] > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 180 ++++++++++++++++++++++- > drivers/gpu/drm/xe/xe_gsc_submit.c | 15 ++ > drivers/gpu/drm/xe/xe_gsc_submit.h | 1 + > 3 files changed, 193 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > index 425db3532ce5..aa8c13916bb6 100644 > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > @@ -4,12 +4,27 @@ > */ > > #include <drm/drm_print.h> > +#include <linux/delay.h> > > +#include "abi/gsc_command_header_abi.h" > #include "intel_hdcp_gsc.h" > #include "xe_device_types.h" > #include "xe_gt.h" > #include "xe_gsc_proxy.h" > #include "xe_pm.h" > +#include "xe_bo.h" > +#include "xe_map.h" > +#include "xe_gsc_submit.h" > + > +#define HECI_MEADDRESS_HDCP 18 > + > +struct intel_hdcp_gsc_message { > + struct xe_bo *hdcp_bo; > + u64 hdcp_cmd_in; > + u64 hdcp_cmd_out; > +}; > + > +#define HDCP_GSC_HEADER_SIZE sizeof(struct intel_gsc_mtl_header) > > bool intel_hdcp_gsc_cs_required(struct xe_device *xe) > { > @@ -40,19 +55,178 @@ bool intel_hdcp_gsc_check_status(struct xe_device *xe) > return ret; > } > > +/*This function helps allocate memory for the command that we will send to gsc cs */ > +static int intel_hdcp_gsc_initialize_message(struct xe_device *xe, > + struct intel_hdcp_gsc_message *hdcp_message) > +{ > + struct xe_bo *bo = NULL; > + u64 cmd_in, cmd_out; > + int ret = 0; > + > + /* allocate object of two page for HDCP command memory and store it */ > + xe_device_mem_access_get(xe); > + bo = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe), NULL, PAGE_SIZE * 2, > + ttm_bo_type_kernel, > + XE_BO_CREATE_SYSTEM_BIT | > + XE_BO_CREATE_GGTT_BIT); > + > + if (IS_ERR(bo)) { > + drm_err(&xe->drm, "Failed to allocate bo for HDCP streaming command!\n"); > + ret = PTR_ERR(bo); > + goto out; > + } > + > + cmd_in = xe_bo_ggtt_addr(bo); > + cmd_out = cmd_in + PAGE_SIZE; > + xe_map_memset(xe, &bo->vmap, 0, 0, bo->size); > + > + hdcp_message->hdcp_bo = bo; > + hdcp_message->hdcp_cmd_in = cmd_in; > + hdcp_message->hdcp_cmd_out = cmd_out; > +out: > + xe_device_mem_access_put(xe); > + return ret; > +} > + > +static int intel_hdcp_gsc_hdcp2_init(struct xe_device *xe) > +{ > + struct intel_hdcp_gsc_message *hdcp_message; > + int ret; > + > + hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL); > + > + if (!hdcp_message) > + return -ENOMEM; > + > + /* > + * NOTE: No need to lock the comp mutex here as it is already > + * going to be taken before this function called > + */ > + ret = intel_hdcp_gsc_initialize_message(xe, hdcp_message); > + xe->display.hdcp.hdcp_message = hdcp_message; > + > + if (ret) { > + drm_err(&xe->drm, "Could not initialize hdcp_message\n"); > + kfree(hdcp_message); Here you're leaving xe->display.hdcp.hdcp_message pointing to a memory location that we no longer own. is that safe for the _fini function? LGTM apart from this (assuming it is going to be squashed with the next patch for merge). Daniele > + } > + > + return ret; > +} > + > int intel_hdcp_gsc_init(struct xe_device *xe) > { > - drm_dbg_kms(&xe->drm, "HDCP support not yet implemented\n"); > - return -ENODEV; > + struct i915_hdcp_arbiter *data; > + int ret; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + mutex_lock(&xe->display.hdcp.hdcp_mutex); > + xe->display.hdcp.arbiter = data; > + xe->display.hdcp.arbiter->hdcp_dev = xe->drm.dev; > + xe->display.hdcp.arbiter->ops = &gsc_hdcp_ops; > + ret = intel_hdcp_gsc_hdcp2_init(xe); > + if (ret) > + kfree(data); > + > + mutex_unlock(&xe->display.hdcp.hdcp_mutex); > + > + return ret; > } > > void intel_hdcp_gsc_fini(struct xe_device *xe) > { > + struct intel_hdcp_gsc_message *hdcp_message = > + xe->display.hdcp.hdcp_message; > + > + if (!hdcp_message) > + return; > + > + xe_bo_unpin_map_no_vm(hdcp_message->hdcp_bo); > + kfree(hdcp_message); > +} > + > +static int xe_gsc_send_sync(struct xe_device *xe, > + struct intel_hdcp_gsc_message *hdcp_message, > + u32 msg_size_in, u32 msg_size_out, > + u32 addr_out_off) > +{ > + struct xe_gt *gt = hdcp_message->hdcp_bo->tile->media_gt; > + struct iosys_map *map = &hdcp_message->hdcp_bo->vmap; > + struct xe_gsc *gsc = >->uc.gsc; > + int ret; > + > + ret = xe_gsc_pkt_submit_kernel(gsc, hdcp_message->hdcp_cmd_in, msg_size_in, > + hdcp_message->hdcp_cmd_out, msg_size_out); > + if (ret) { > + drm_err(&xe->drm, "failed to send gsc HDCP msg (%d)\n", ret); > + return ret; > + } > + > + if (xe_gsc_check_and_update_pending(xe, map, 0, map, addr_out_off)) > + return -EAGAIN; > + > + ret = xe_gsc_read_out_header(xe, map, addr_out_off, > + sizeof(struct hdcp_cmd_header), NULL); > + > + return ret; > } > > ssize_t intel_hdcp_gsc_msg_send(struct xe_device *xe, u8 *msg_in, > size_t msg_in_len, u8 *msg_out, > size_t msg_out_len) > { > - return -ENODEV; > + const size_t max_msg_size = PAGE_SIZE - HDCP_GSC_HEADER_SIZE; > + struct intel_hdcp_gsc_message *hdcp_message; > + u64 host_session_id; > + u32 msg_size_in, msg_size_out; > + u32 addr_out_off, addr_in_wr_off = 0; > + int ret, tries = 0; > + > + if (msg_in_len > max_msg_size || msg_out_len > max_msg_size) { > + ret = -ENOSPC; > + goto out; > + } > + > + msg_size_in = msg_in_len + HDCP_GSC_HEADER_SIZE; > + msg_size_out = msg_out_len + HDCP_GSC_HEADER_SIZE; > + hdcp_message = xe->display.hdcp.hdcp_message; > + addr_out_off = PAGE_SIZE; > + > + host_session_id = xe_gsc_create_host_session_id(); > + xe_device_mem_access_get(xe); > + addr_in_wr_off = xe_gsc_emit_header(xe, &hdcp_message->hdcp_bo->vmap, > + addr_in_wr_off, HECI_MEADDRESS_HDCP, > + host_session_id, msg_in_len); > + xe_map_memcpy_to(xe, &hdcp_message->hdcp_bo->vmap, addr_in_wr_off, > + msg_in, msg_in_len); > + /* > + * Keep sending request in case the pending bit is set no need to add > + * message handle as we are using same address hence loc. of header is > + * same and it will contain the message handle. we will send the message > + * 20 times each message 50 ms apart > + */ > + do { > + ret = xe_gsc_send_sync(xe, hdcp_message, msg_size_in, msg_size_out, > + addr_out_off); > + > + /* Only try again if gsc says so */ > + if (ret != -EAGAIN) > + break; > + > + msleep(50); > + > + } while (++tries < 20); > + > + if (ret) > + goto out; > + > + xe_map_memcpy_from(xe, msg_out, &hdcp_message->hdcp_bo->vmap, > + addr_out_off + HDCP_GSC_HEADER_SIZE, > + msg_out_len); > + > +out: > + xe_device_mem_access_put(xe); > + return ret; > } > diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.c b/drivers/gpu/drm/xe/xe_gsc_submit.c > index 348994b271be..9a18f5667db3 100644 > --- a/drivers/gpu/drm/xe/xe_gsc_submit.c > +++ b/drivers/gpu/drm/xe/xe_gsc_submit.c > @@ -40,6 +40,21 @@ gsc_to_gt(struct xe_gsc *gsc) > return container_of(gsc, struct xe_gt, uc.gsc); > } > > +/** > + * xe_gsc_get_host_session_id - Creates a random 64 bit host_session id with > + * bits 56-63 masked. > + * > + * Returns: random host_session_id which can be used to send messages to gsc cs > + */ > +u64 xe_gsc_create_host_session_id(void) > +{ > + u64 host_session_id; > + > + get_random_bytes(&host_session_id, sizeof(u64)); > + host_session_id &= ~HOST_SESSION_CLIENT_MASK; > + return host_session_id; > +} > + > /** > * xe_gsc_emit_header - write the MTL GSC header in memory > * @xe: the Xe device > diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.h b/drivers/gpu/drm/xe/xe_gsc_submit.h > index 1939855031a6..1416b5745a4c 100644 > --- a/drivers/gpu/drm/xe/xe_gsc_submit.h > +++ b/drivers/gpu/drm/xe/xe_gsc_submit.h > @@ -28,4 +28,5 @@ int xe_gsc_read_out_header(struct xe_device *xe, > int xe_gsc_pkt_submit_kernel(struct xe_gsc *gsc, u64 addr_in, u32 size_in, > u64 addr_out, u32 size_out); > > +u64 xe_gsc_create_host_session_id(void); > #endif
> > interaction of HDCP as a client with the GSC CS interface. > > > > --v2 > > -add kfree at appropriate place [Daniele] -remove useless define > > [Daniele] -move host session logic to xe_gsc_submit.c [Daniele] -call > > xe_gsc_check_and_update_pending directly in an if condition [Daniele] > > -use xe_device instead of drm_i915_private [Daniele] > > > > --v3 > > -use xe prefix for newly exposed function [Daniele] -remove client > > specific defines from intel_gsc_mtl_header [Daniele] -add missing > > kfree() [Daniele] -have NULL check for hdcp_message in finish function > > [Daniele] -dont have too many variable declarations in the same line > > [Daniele] > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 180 > ++++++++++++++++++++++- > > drivers/gpu/drm/xe/xe_gsc_submit.c | 15 ++ > > drivers/gpu/drm/xe/xe_gsc_submit.h | 1 + > > 3 files changed, 193 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > index 425db3532ce5..aa8c13916bb6 100644 > > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > @@ -4,12 +4,27 @@ > > */ > > > > #include <drm/drm_print.h> > > +#include <linux/delay.h> > > > > +#include "abi/gsc_command_header_abi.h" > > #include "intel_hdcp_gsc.h" > > #include "xe_device_types.h" > > #include "xe_gt.h" > > #include "xe_gsc_proxy.h" > > #include "xe_pm.h" > > +#include "xe_bo.h" > > +#include "xe_map.h" > > +#include "xe_gsc_submit.h" > > + > > +#define HECI_MEADDRESS_HDCP 18 > > + > > +struct intel_hdcp_gsc_message { > > + struct xe_bo *hdcp_bo; > > + u64 hdcp_cmd_in; > > + u64 hdcp_cmd_out; > > +}; > > + > > +#define HDCP_GSC_HEADER_SIZE sizeof(struct intel_gsc_mtl_header) > > > > bool intel_hdcp_gsc_cs_required(struct xe_device *xe) > > { > > @@ -40,19 +55,178 @@ bool intel_hdcp_gsc_check_status(struct xe_device > *xe) > > return ret; > > } > > > > +/*This function helps allocate memory for the command that we will > > +send to gsc cs */ static int intel_hdcp_gsc_initialize_message(struct > xe_device *xe, > > + struct intel_hdcp_gsc_message > *hdcp_message) { > > + struct xe_bo *bo = NULL; > > + u64 cmd_in, cmd_out; > > + int ret = 0; > > + > > + /* allocate object of two page for HDCP command memory and store > it */ > > + xe_device_mem_access_get(xe); > > + bo = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe), NULL, > PAGE_SIZE * 2, > > + ttm_bo_type_kernel, > > + XE_BO_CREATE_SYSTEM_BIT | > > + XE_BO_CREATE_GGTT_BIT); > > + > > + if (IS_ERR(bo)) { > > + drm_err(&xe->drm, "Failed to allocate bo for HDCP streaming > command!\n"); > > + ret = PTR_ERR(bo); > > + goto out; > > + } > > + > > + cmd_in = xe_bo_ggtt_addr(bo); > > + cmd_out = cmd_in + PAGE_SIZE; > > + xe_map_memset(xe, &bo->vmap, 0, 0, bo->size); > > + > > + hdcp_message->hdcp_bo = bo; > > + hdcp_message->hdcp_cmd_in = cmd_in; > > + hdcp_message->hdcp_cmd_out = cmd_out; > > +out: > > + xe_device_mem_access_put(xe); > > + return ret; > > +} > > + > > +static int intel_hdcp_gsc_hdcp2_init(struct xe_device *xe) { > > + struct intel_hdcp_gsc_message *hdcp_message; > > + int ret; > > + > > + hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL); > > + > > + if (!hdcp_message) > > + return -ENOMEM; > > + > > + /* > > + * NOTE: No need to lock the comp mutex here as it is already > > + * going to be taken before this function called > > + */ > > + ret = intel_hdcp_gsc_initialize_message(xe, hdcp_message); > > + xe->display.hdcp.hdcp_message = hdcp_message; > > + > > + if (ret) { > > + drm_err(&xe->drm, "Could not initialize hdcp_message\n"); > > + kfree(hdcp_message); > > Here you're leaving xe->display.hdcp.hdcp_message pointing to a memory > location that we no longer own. is that safe for the _fini function? > Would it be better to not have a kfree here if initialization fails it gets taken care of In finish function anyways that would be safer I presume. > LGTM apart from this (assuming it is going to be squashed with the next > patch for merge). Yes this will be squashed with the next patch when I send the new revision Regards, Suraj Kandpal > > Daniele > > > + } > > + > > + return ret; > > +} > > + > > int intel_hdcp_gsc_init(struct xe_device *xe) > > { > > - drm_dbg_kms(&xe->drm, "HDCP support not yet implemented\n"); > > - return -ENODEV; > > + struct i915_hdcp_arbiter *data; > > + int ret; > > + > > + data = kzalloc(sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + mutex_lock(&xe->display.hdcp.hdcp_mutex); > > + xe->display.hdcp.arbiter = data; > > + xe->display.hdcp.arbiter->hdcp_dev = xe->drm.dev; > > + xe->display.hdcp.arbiter->ops = &gsc_hdcp_ops; > > + ret = intel_hdcp_gsc_hdcp2_init(xe); > > + if (ret) > > + kfree(data); > > + > > + mutex_unlock(&xe->display.hdcp.hdcp_mutex); > > + > > + return ret; > > } > > > > void intel_hdcp_gsc_fini(struct xe_device *xe) > > { > > + struct intel_hdcp_gsc_message *hdcp_message = > > + xe->display.hdcp.hdcp_message; > > + > > + if (!hdcp_message) > > + return; > > + > > + xe_bo_unpin_map_no_vm(hdcp_message->hdcp_bo); > > + kfree(hdcp_message); > > +} > > + > > +static int xe_gsc_send_sync(struct xe_device *xe, > > + struct intel_hdcp_gsc_message *hdcp_message, > > + u32 msg_size_in, u32 msg_size_out, > > + u32 addr_out_off) > > +{ > > + struct xe_gt *gt = hdcp_message->hdcp_bo->tile->media_gt; > > + struct iosys_map *map = &hdcp_message->hdcp_bo->vmap; > > + struct xe_gsc *gsc = >->uc.gsc; > > + int ret; > > + > > + ret = xe_gsc_pkt_submit_kernel(gsc, hdcp_message->hdcp_cmd_in, > msg_size_in, > > + hdcp_message->hdcp_cmd_out, > msg_size_out); > > + if (ret) { > > + drm_err(&xe->drm, "failed to send gsc HDCP msg (%d)\n", > ret); > > + return ret; > > + } > > + > > + if (xe_gsc_check_and_update_pending(xe, map, 0, map, > addr_out_off)) > > + return -EAGAIN; > > + > > + ret = xe_gsc_read_out_header(xe, map, addr_out_off, > > + sizeof(struct hdcp_cmd_header), NULL); > > + > > + return ret; > > } > > > > ssize_t intel_hdcp_gsc_msg_send(struct xe_device *xe, u8 *msg_in, > > size_t msg_in_len, u8 *msg_out, > > size_t msg_out_len) > > { > > - return -ENODEV; > > + const size_t max_msg_size = PAGE_SIZE - HDCP_GSC_HEADER_SIZE; > > + struct intel_hdcp_gsc_message *hdcp_message; > > + u64 host_session_id; > > + u32 msg_size_in, msg_size_out; > > + u32 addr_out_off, addr_in_wr_off = 0; > > + int ret, tries = 0; > > + > > + if (msg_in_len > max_msg_size || msg_out_len > max_msg_size) { > > + ret = -ENOSPC; > > + goto out; > > + } > > + > > + msg_size_in = msg_in_len + HDCP_GSC_HEADER_SIZE; > > + msg_size_out = msg_out_len + HDCP_GSC_HEADER_SIZE; > > + hdcp_message = xe->display.hdcp.hdcp_message; > > + addr_out_off = PAGE_SIZE; > > + > > + host_session_id = xe_gsc_create_host_session_id(); > > + xe_device_mem_access_get(xe); > > + addr_in_wr_off = xe_gsc_emit_header(xe, &hdcp_message- > >hdcp_bo->vmap, > > + addr_in_wr_off, > HECI_MEADDRESS_HDCP, > > + host_session_id, msg_in_len); > > + xe_map_memcpy_to(xe, &hdcp_message->hdcp_bo->vmap, > addr_in_wr_off, > > + msg_in, msg_in_len); > > + /* > > + * Keep sending request in case the pending bit is set no need to add > > + * message handle as we are using same address hence loc. of header > is > > + * same and it will contain the message handle. we will send the > message > > + * 20 times each message 50 ms apart > > + */ > > + do { > > + ret = xe_gsc_send_sync(xe, hdcp_message, msg_size_in, > msg_size_out, > > + addr_out_off); > > + > > + /* Only try again if gsc says so */ > > + if (ret != -EAGAIN) > > + break; > > + > > + msleep(50); > > + > > + } while (++tries < 20); > > + > > + if (ret) > > + goto out; > > + > > + xe_map_memcpy_from(xe, msg_out, &hdcp_message->hdcp_bo- > >vmap, > > + addr_out_off + HDCP_GSC_HEADER_SIZE, > > + msg_out_len); > > + > > +out: > > + xe_device_mem_access_put(xe); > > + return ret; > > } > > diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.c > > b/drivers/gpu/drm/xe/xe_gsc_submit.c > > index 348994b271be..9a18f5667db3 100644 > > --- a/drivers/gpu/drm/xe/xe_gsc_submit.c > > +++ b/drivers/gpu/drm/xe/xe_gsc_submit.c > > @@ -40,6 +40,21 @@ gsc_to_gt(struct xe_gsc *gsc) > > return container_of(gsc, struct xe_gt, uc.gsc); > > } > > > > +/** > > + * xe_gsc_get_host_session_id - Creates a random 64 bit host_session > > +id with > > + * bits 56-63 masked. > > + * > > + * Returns: random host_session_id which can be used to send messages > > +to gsc cs */ > > +u64 xe_gsc_create_host_session_id(void) > > +{ > > + u64 host_session_id; > > + > > + get_random_bytes(&host_session_id, sizeof(u64)); > > + host_session_id &= ~HOST_SESSION_CLIENT_MASK; > > + return host_session_id; > > +} > > + > > /** > > * xe_gsc_emit_header - write the MTL GSC header in memory > > * @xe: the Xe device > > diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.h > > b/drivers/gpu/drm/xe/xe_gsc_submit.h > > index 1939855031a6..1416b5745a4c 100644 > > --- a/drivers/gpu/drm/xe/xe_gsc_submit.h > > +++ b/drivers/gpu/drm/xe/xe_gsc_submit.h > > @@ -28,4 +28,5 @@ int xe_gsc_read_out_header(struct xe_device *xe, > > int xe_gsc_pkt_submit_kernel(struct xe_gsc *gsc, u64 addr_in, u32 size_in, > > u64 addr_out, u32 size_out); > > > > +u64 xe_gsc_create_host_session_id(void); > > #endif
On 2/13/2024 9:33 PM, Kandpal, Suraj wrote: >>> interaction of HDCP as a client with the GSC CS interface. >>> >>> --v2 >>> -add kfree at appropriate place [Daniele] -remove useless define >>> [Daniele] -move host session logic to xe_gsc_submit.c [Daniele] -call >>> xe_gsc_check_and_update_pending directly in an if condition [Daniele] >>> -use xe_device instead of drm_i915_private [Daniele] >>> >>> --v3 >>> -use xe prefix for newly exposed function [Daniele] -remove client >>> specific defines from intel_gsc_mtl_header [Daniele] -add missing >>> kfree() [Daniele] -have NULL check for hdcp_message in finish function >>> [Daniele] -dont have too many variable declarations in the same line >>> [Daniele] >>> >>> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> >>> --- >>> drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 180 >> ++++++++++++++++++++++- >>> drivers/gpu/drm/xe/xe_gsc_submit.c | 15 ++ >>> drivers/gpu/drm/xe/xe_gsc_submit.h | 1 + >>> 3 files changed, 193 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> index 425db3532ce5..aa8c13916bb6 100644 >>> --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> @@ -4,12 +4,27 @@ >>> */ >>> >>> #include <drm/drm_print.h> >>> +#include <linux/delay.h> >>> >>> +#include "abi/gsc_command_header_abi.h" >>> #include "intel_hdcp_gsc.h" >>> #include "xe_device_types.h" >>> #include "xe_gt.h" >>> #include "xe_gsc_proxy.h" >>> #include "xe_pm.h" >>> +#include "xe_bo.h" >>> +#include "xe_map.h" >>> +#include "xe_gsc_submit.h" >>> + >>> +#define HECI_MEADDRESS_HDCP 18 >>> + >>> +struct intel_hdcp_gsc_message { >>> + struct xe_bo *hdcp_bo; >>> + u64 hdcp_cmd_in; >>> + u64 hdcp_cmd_out; >>> +}; >>> + >>> +#define HDCP_GSC_HEADER_SIZE sizeof(struct intel_gsc_mtl_header) >>> >>> bool intel_hdcp_gsc_cs_required(struct xe_device *xe) >>> { >>> @@ -40,19 +55,178 @@ bool intel_hdcp_gsc_check_status(struct xe_device >> *xe) >>> return ret; >>> } >>> >>> +/*This function helps allocate memory for the command that we will >>> +send to gsc cs */ static int intel_hdcp_gsc_initialize_message(struct >> xe_device *xe, >>> + struct intel_hdcp_gsc_message >> *hdcp_message) { >>> + struct xe_bo *bo = NULL; >>> + u64 cmd_in, cmd_out; >>> + int ret = 0; >>> + >>> + /* allocate object of two page for HDCP command memory and store >> it */ >>> + xe_device_mem_access_get(xe); >>> + bo = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe), NULL, >> PAGE_SIZE * 2, >>> + ttm_bo_type_kernel, >>> + XE_BO_CREATE_SYSTEM_BIT | >>> + XE_BO_CREATE_GGTT_BIT); >>> + >>> + if (IS_ERR(bo)) { >>> + drm_err(&xe->drm, "Failed to allocate bo for HDCP streaming >> command!\n"); >>> + ret = PTR_ERR(bo); >>> + goto out; >>> + } >>> + >>> + cmd_in = xe_bo_ggtt_addr(bo); >>> + cmd_out = cmd_in + PAGE_SIZE; >>> + xe_map_memset(xe, &bo->vmap, 0, 0, bo->size); >>> + >>> + hdcp_message->hdcp_bo = bo; >>> + hdcp_message->hdcp_cmd_in = cmd_in; >>> + hdcp_message->hdcp_cmd_out = cmd_out; >>> +out: >>> + xe_device_mem_access_put(xe); >>> + return ret; >>> +} >>> + >>> +static int intel_hdcp_gsc_hdcp2_init(struct xe_device *xe) { >>> + struct intel_hdcp_gsc_message *hdcp_message; >>> + int ret; >>> + >>> + hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL); >>> + >>> + if (!hdcp_message) >>> + return -ENOMEM; >>> + >>> + /* >>> + * NOTE: No need to lock the comp mutex here as it is already >>> + * going to be taken before this function called >>> + */ >>> + ret = intel_hdcp_gsc_initialize_message(xe, hdcp_message); >>> + xe->display.hdcp.hdcp_message = hdcp_message; >>> + >>> + if (ret) { >>> + drm_err(&xe->drm, "Could not initialize hdcp_message\n"); >>> + kfree(hdcp_message); >> Here you're leaving xe->display.hdcp.hdcp_message pointing to a memory >> location that we no longer own. is that safe for the _fini function? >> > Would it be better to not have a kfree here if initialization fails it gets taken care of > In finish function anyways that would be safer I presume. We normally try to clean up immediately when things go wrong, also because if this failure causes the driver load to abort the _fini function might not get called (but not sure if this is the case here). In this case this can be easily fixed by simply changing it to: ret = intel_hdcp_gsc_initialize_message(xe, hdcp_message); if (ret) { drm_err(&xe->drm, "Could not initialize hdcp_message\n"); kfree(hdcp_message); return ret; } xe->display.hdcp.hdcp_message = hdcp_message; return 0; Which guarantees that xe->display.hdcp.hdcp_message is only set when the allocation exists with minimal changes to the code. Daniele > >> LGTM apart from this (assuming it is going to be squashed with the next >> patch for merge). > Yes this will be squashed with the next patch when I send the new revision > > Regards, > Suraj Kandpal >> Daniele >> >>> + } >>> + >>> + return ret; >>> +} >>> + >>> int intel_hdcp_gsc_init(struct xe_device *xe) >>> { >>> - drm_dbg_kms(&xe->drm, "HDCP support not yet implemented\n"); >>> - return -ENODEV; >>> + struct i915_hdcp_arbiter *data; >>> + int ret; >>> + >>> + data = kzalloc(sizeof(*data), GFP_KERNEL); >>> + if (!data) >>> + return -ENOMEM; >>> + >>> + mutex_lock(&xe->display.hdcp.hdcp_mutex); >>> + xe->display.hdcp.arbiter = data; >>> + xe->display.hdcp.arbiter->hdcp_dev = xe->drm.dev; >>> + xe->display.hdcp.arbiter->ops = &gsc_hdcp_ops; >>> + ret = intel_hdcp_gsc_hdcp2_init(xe); >>> + if (ret) >>> + kfree(data); >>> + >>> + mutex_unlock(&xe->display.hdcp.hdcp_mutex); >>> + >>> + return ret; >>> } >>> >>> void intel_hdcp_gsc_fini(struct xe_device *xe) >>> { >>> + struct intel_hdcp_gsc_message *hdcp_message = >>> + xe->display.hdcp.hdcp_message; >>> + >>> + if (!hdcp_message) >>> + return; >>> + >>> + xe_bo_unpin_map_no_vm(hdcp_message->hdcp_bo); >>> + kfree(hdcp_message); >>> +} >>> + >>> +static int xe_gsc_send_sync(struct xe_device *xe, >>> + struct intel_hdcp_gsc_message *hdcp_message, >>> + u32 msg_size_in, u32 msg_size_out, >>> + u32 addr_out_off) >>> +{ >>> + struct xe_gt *gt = hdcp_message->hdcp_bo->tile->media_gt; >>> + struct iosys_map *map = &hdcp_message->hdcp_bo->vmap; >>> + struct xe_gsc *gsc = >->uc.gsc; >>> + int ret; >>> + >>> + ret = xe_gsc_pkt_submit_kernel(gsc, hdcp_message->hdcp_cmd_in, >> msg_size_in, >>> + hdcp_message->hdcp_cmd_out, >> msg_size_out); >>> + if (ret) { >>> + drm_err(&xe->drm, "failed to send gsc HDCP msg (%d)\n", >> ret); >>> + return ret; >>> + } >>> + >>> + if (xe_gsc_check_and_update_pending(xe, map, 0, map, >> addr_out_off)) >>> + return -EAGAIN; >>> + >>> + ret = xe_gsc_read_out_header(xe, map, addr_out_off, >>> + sizeof(struct hdcp_cmd_header), NULL); >>> + >>> + return ret; >>> } >>> >>> ssize_t intel_hdcp_gsc_msg_send(struct xe_device *xe, u8 *msg_in, >>> size_t msg_in_len, u8 *msg_out, >>> size_t msg_out_len) >>> { >>> - return -ENODEV; >>> + const size_t max_msg_size = PAGE_SIZE - HDCP_GSC_HEADER_SIZE; >>> + struct intel_hdcp_gsc_message *hdcp_message; >>> + u64 host_session_id; >>> + u32 msg_size_in, msg_size_out; >>> + u32 addr_out_off, addr_in_wr_off = 0; >>> + int ret, tries = 0; >>> + >>> + if (msg_in_len > max_msg_size || msg_out_len > max_msg_size) { >>> + ret = -ENOSPC; >>> + goto out; >>> + } >>> + >>> + msg_size_in = msg_in_len + HDCP_GSC_HEADER_SIZE; >>> + msg_size_out = msg_out_len + HDCP_GSC_HEADER_SIZE; >>> + hdcp_message = xe->display.hdcp.hdcp_message; >>> + addr_out_off = PAGE_SIZE; >>> + >>> + host_session_id = xe_gsc_create_host_session_id(); >>> + xe_device_mem_access_get(xe); >>> + addr_in_wr_off = xe_gsc_emit_header(xe, &hdcp_message- >>> hdcp_bo->vmap, >>> + addr_in_wr_off, >> HECI_MEADDRESS_HDCP, >>> + host_session_id, msg_in_len); >>> + xe_map_memcpy_to(xe, &hdcp_message->hdcp_bo->vmap, >> addr_in_wr_off, >>> + msg_in, msg_in_len); >>> + /* >>> + * Keep sending request in case the pending bit is set no need to add >>> + * message handle as we are using same address hence loc. of header >> is >>> + * same and it will contain the message handle. we will send the >> message >>> + * 20 times each message 50 ms apart >>> + */ >>> + do { >>> + ret = xe_gsc_send_sync(xe, hdcp_message, msg_size_in, >> msg_size_out, >>> + addr_out_off); >>> + >>> + /* Only try again if gsc says so */ >>> + if (ret != -EAGAIN) >>> + break; >>> + >>> + msleep(50); >>> + >>> + } while (++tries < 20); >>> + >>> + if (ret) >>> + goto out; >>> + >>> + xe_map_memcpy_from(xe, msg_out, &hdcp_message->hdcp_bo- >>> vmap, >>> + addr_out_off + HDCP_GSC_HEADER_SIZE, >>> + msg_out_len); >>> + >>> +out: >>> + xe_device_mem_access_put(xe); >>> + return ret; >>> } >>> diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.c >>> b/drivers/gpu/drm/xe/xe_gsc_submit.c >>> index 348994b271be..9a18f5667db3 100644 >>> --- a/drivers/gpu/drm/xe/xe_gsc_submit.c >>> +++ b/drivers/gpu/drm/xe/xe_gsc_submit.c >>> @@ -40,6 +40,21 @@ gsc_to_gt(struct xe_gsc *gsc) >>> return container_of(gsc, struct xe_gt, uc.gsc); >>> } >>> >>> +/** >>> + * xe_gsc_get_host_session_id - Creates a random 64 bit host_session >>> +id with >>> + * bits 56-63 masked. >>> + * >>> + * Returns: random host_session_id which can be used to send messages >>> +to gsc cs */ >>> +u64 xe_gsc_create_host_session_id(void) >>> +{ >>> + u64 host_session_id; >>> + >>> + get_random_bytes(&host_session_id, sizeof(u64)); >>> + host_session_id &= ~HOST_SESSION_CLIENT_MASK; >>> + return host_session_id; >>> +} >>> + >>> /** >>> * xe_gsc_emit_header - write the MTL GSC header in memory >>> * @xe: the Xe device >>> diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.h >>> b/drivers/gpu/drm/xe/xe_gsc_submit.h >>> index 1939855031a6..1416b5745a4c 100644 >>> --- a/drivers/gpu/drm/xe/xe_gsc_submit.h >>> +++ b/drivers/gpu/drm/xe/xe_gsc_submit.h >>> @@ -28,4 +28,5 @@ int xe_gsc_read_out_header(struct xe_device *xe, >>> int xe_gsc_pkt_submit_kernel(struct xe_gsc *gsc, u64 addr_in, u32 size_in, >>> u64 addr_out, u32 size_out); >>> >>> +u64 xe_gsc_create_host_session_id(void); >>> #endif
diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c index 425db3532ce5..aa8c13916bb6 100644 --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c @@ -4,12 +4,27 @@ */ #include <drm/drm_print.h> +#include <linux/delay.h> +#include "abi/gsc_command_header_abi.h" #include "intel_hdcp_gsc.h" #include "xe_device_types.h" #include "xe_gt.h" #include "xe_gsc_proxy.h" #include "xe_pm.h" +#include "xe_bo.h" +#include "xe_map.h" +#include "xe_gsc_submit.h" + +#define HECI_MEADDRESS_HDCP 18 + +struct intel_hdcp_gsc_message { + struct xe_bo *hdcp_bo; + u64 hdcp_cmd_in; + u64 hdcp_cmd_out; +}; + +#define HDCP_GSC_HEADER_SIZE sizeof(struct intel_gsc_mtl_header) bool intel_hdcp_gsc_cs_required(struct xe_device *xe) { @@ -40,19 +55,178 @@ bool intel_hdcp_gsc_check_status(struct xe_device *xe) return ret; } +/*This function helps allocate memory for the command that we will send to gsc cs */ +static int intel_hdcp_gsc_initialize_message(struct xe_device *xe, + struct intel_hdcp_gsc_message *hdcp_message) +{ + struct xe_bo *bo = NULL; + u64 cmd_in, cmd_out; + int ret = 0; + + /* allocate object of two page for HDCP command memory and store it */ + xe_device_mem_access_get(xe); + bo = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe), NULL, PAGE_SIZE * 2, + ttm_bo_type_kernel, + XE_BO_CREATE_SYSTEM_BIT | + XE_BO_CREATE_GGTT_BIT); + + if (IS_ERR(bo)) { + drm_err(&xe->drm, "Failed to allocate bo for HDCP streaming command!\n"); + ret = PTR_ERR(bo); + goto out; + } + + cmd_in = xe_bo_ggtt_addr(bo); + cmd_out = cmd_in + PAGE_SIZE; + xe_map_memset(xe, &bo->vmap, 0, 0, bo->size); + + hdcp_message->hdcp_bo = bo; + hdcp_message->hdcp_cmd_in = cmd_in; + hdcp_message->hdcp_cmd_out = cmd_out; +out: + xe_device_mem_access_put(xe); + return ret; +} + +static int intel_hdcp_gsc_hdcp2_init(struct xe_device *xe) +{ + struct intel_hdcp_gsc_message *hdcp_message; + int ret; + + hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL); + + if (!hdcp_message) + return -ENOMEM; + + /* + * NOTE: No need to lock the comp mutex here as it is already + * going to be taken before this function called + */ + ret = intel_hdcp_gsc_initialize_message(xe, hdcp_message); + xe->display.hdcp.hdcp_message = hdcp_message; + + if (ret) { + drm_err(&xe->drm, "Could not initialize hdcp_message\n"); + kfree(hdcp_message); + } + + return ret; +} + int intel_hdcp_gsc_init(struct xe_device *xe) { - drm_dbg_kms(&xe->drm, "HDCP support not yet implemented\n"); - return -ENODEV; + struct i915_hdcp_arbiter *data; + int ret; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + mutex_lock(&xe->display.hdcp.hdcp_mutex); + xe->display.hdcp.arbiter = data; + xe->display.hdcp.arbiter->hdcp_dev = xe->drm.dev; + xe->display.hdcp.arbiter->ops = &gsc_hdcp_ops; + ret = intel_hdcp_gsc_hdcp2_init(xe); + if (ret) + kfree(data); + + mutex_unlock(&xe->display.hdcp.hdcp_mutex); + + return ret; } void intel_hdcp_gsc_fini(struct xe_device *xe) { + struct intel_hdcp_gsc_message *hdcp_message = + xe->display.hdcp.hdcp_message; + + if (!hdcp_message) + return; + + xe_bo_unpin_map_no_vm(hdcp_message->hdcp_bo); + kfree(hdcp_message); +} + +static int xe_gsc_send_sync(struct xe_device *xe, + struct intel_hdcp_gsc_message *hdcp_message, + u32 msg_size_in, u32 msg_size_out, + u32 addr_out_off) +{ + struct xe_gt *gt = hdcp_message->hdcp_bo->tile->media_gt; + struct iosys_map *map = &hdcp_message->hdcp_bo->vmap; + struct xe_gsc *gsc = >->uc.gsc; + int ret; + + ret = xe_gsc_pkt_submit_kernel(gsc, hdcp_message->hdcp_cmd_in, msg_size_in, + hdcp_message->hdcp_cmd_out, msg_size_out); + if (ret) { + drm_err(&xe->drm, "failed to send gsc HDCP msg (%d)\n", ret); + return ret; + } + + if (xe_gsc_check_and_update_pending(xe, map, 0, map, addr_out_off)) + return -EAGAIN; + + ret = xe_gsc_read_out_header(xe, map, addr_out_off, + sizeof(struct hdcp_cmd_header), NULL); + + return ret; } ssize_t intel_hdcp_gsc_msg_send(struct xe_device *xe, u8 *msg_in, size_t msg_in_len, u8 *msg_out, size_t msg_out_len) { - return -ENODEV; + const size_t max_msg_size = PAGE_SIZE - HDCP_GSC_HEADER_SIZE; + struct intel_hdcp_gsc_message *hdcp_message; + u64 host_session_id; + u32 msg_size_in, msg_size_out; + u32 addr_out_off, addr_in_wr_off = 0; + int ret, tries = 0; + + if (msg_in_len > max_msg_size || msg_out_len > max_msg_size) { + ret = -ENOSPC; + goto out; + } + + msg_size_in = msg_in_len + HDCP_GSC_HEADER_SIZE; + msg_size_out = msg_out_len + HDCP_GSC_HEADER_SIZE; + hdcp_message = xe->display.hdcp.hdcp_message; + addr_out_off = PAGE_SIZE; + + host_session_id = xe_gsc_create_host_session_id(); + xe_device_mem_access_get(xe); + addr_in_wr_off = xe_gsc_emit_header(xe, &hdcp_message->hdcp_bo->vmap, + addr_in_wr_off, HECI_MEADDRESS_HDCP, + host_session_id, msg_in_len); + xe_map_memcpy_to(xe, &hdcp_message->hdcp_bo->vmap, addr_in_wr_off, + msg_in, msg_in_len); + /* + * Keep sending request in case the pending bit is set no need to add + * message handle as we are using same address hence loc. of header is + * same and it will contain the message handle. we will send the message + * 20 times each message 50 ms apart + */ + do { + ret = xe_gsc_send_sync(xe, hdcp_message, msg_size_in, msg_size_out, + addr_out_off); + + /* Only try again if gsc says so */ + if (ret != -EAGAIN) + break; + + msleep(50); + + } while (++tries < 20); + + if (ret) + goto out; + + xe_map_memcpy_from(xe, msg_out, &hdcp_message->hdcp_bo->vmap, + addr_out_off + HDCP_GSC_HEADER_SIZE, + msg_out_len); + +out: + xe_device_mem_access_put(xe); + return ret; } diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.c b/drivers/gpu/drm/xe/xe_gsc_submit.c index 348994b271be..9a18f5667db3 100644 --- a/drivers/gpu/drm/xe/xe_gsc_submit.c +++ b/drivers/gpu/drm/xe/xe_gsc_submit.c @@ -40,6 +40,21 @@ gsc_to_gt(struct xe_gsc *gsc) return container_of(gsc, struct xe_gt, uc.gsc); } +/** + * xe_gsc_get_host_session_id - Creates a random 64 bit host_session id with + * bits 56-63 masked. + * + * Returns: random host_session_id which can be used to send messages to gsc cs + */ +u64 xe_gsc_create_host_session_id(void) +{ + u64 host_session_id; + + get_random_bytes(&host_session_id, sizeof(u64)); + host_session_id &= ~HOST_SESSION_CLIENT_MASK; + return host_session_id; +} + /** * xe_gsc_emit_header - write the MTL GSC header in memory * @xe: the Xe device diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.h b/drivers/gpu/drm/xe/xe_gsc_submit.h index 1939855031a6..1416b5745a4c 100644 --- a/drivers/gpu/drm/xe/xe_gsc_submit.h +++ b/drivers/gpu/drm/xe/xe_gsc_submit.h @@ -28,4 +28,5 @@ int xe_gsc_read_out_header(struct xe_device *xe, int xe_gsc_pkt_submit_kernel(struct xe_gsc *gsc, u64 addr_in, u32 size_in, u64 addr_out, u32 size_out); +u64 xe_gsc_create_host_session_id(void); #endif
Enable HDCP for Xe by defining functions which take care of interaction of HDCP as a client with the GSC CS interface. --v2 -add kfree at appropriate place [Daniele] -remove useless define [Daniele] -move host session logic to xe_gsc_submit.c [Daniele] -call xe_gsc_check_and_update_pending directly in an if condition [Daniele] -use xe_device instead of drm_i915_private [Daniele] --v3 -use xe prefix for newly exposed function [Daniele] -remove client specific defines from intel_gsc_mtl_header [Daniele] -add missing kfree() [Daniele] -have NULL check for hdcp_message in finish function [Daniele] -dont have too many variable declarations in the same line [Daniele] Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 180 ++++++++++++++++++++++- drivers/gpu/drm/xe/xe_gsc_submit.c | 15 ++ drivers/gpu/drm/xe/xe_gsc_submit.h | 1 + 3 files changed, 193 insertions(+), 3 deletions(-)